From: Lin Yang <lin.a.yang@intel.com>

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@intel.com>
---
 src/qemu/qemu_alias.c                         |  3 +-
 src/qemu/qemu_command.c                       | 46 +++++++++++++++++--
 .../sgx-epc.x86_64-6.2.0.args                 | 38 +++++++++++++++
 tests/qemuxml2argvtest.c                      |  2 +
 4 files changed, 84 insertions(+), 5 deletions(-)
 create mode 100644 tests/qemuxml2argvdata/sgx-epc.x86_64-6.2.0.args

diff --git a/src/qemu/qemu_alias.c b/src/qemu/qemu_alias.c
index e5a946cbed..03c79bcf0e 100644
--- a/src/qemu/qemu_alias.c
+++ b/src/qemu/qemu_alias.c
@@ -467,7 +467,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 e6f29d878c..71804e659c 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -3815,6 +3815,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 if (useHugepage || mem->nvdimmPath || memAccess ||
         def->mem.source == VIR_DOMAIN_MEMORY_SOURCE_FILE) {
 
@@ -3972,6 +3976,12 @@ qemuBuildMemoryBackendProps(virJSONValue **backendProps,
                            _("this qemu doesn't support the "
                              "memory-backend-memfd object"));
             return -1;
+        } else if (STREQ(backendType, "memory-backend-epc") &&
+                   !virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_SGX_EPC)) {
+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+                           _("this qemu doesn't support the "
+                             "memory-backend-epc object"));
+            return -1;
         }
 
         rc = 0;
@@ -7800,6 +7810,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 */
@@ -7809,11 +7821,37 @@ qemuBuildMemoryDeviceCommandLine(virCommand *cmd,
         if (qemuBuildMemoryDimmBackendStr(cmd, def->mems[i], def, cfg, priv) < 0)
             return -1;
 
-        if (!(props = qemuBuildMemoryDeviceProps(cfg, priv, 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(cfg, priv, 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=mem%s", epcNum++,
+                              def->mems[i]->info.alias);
+
+            break;
+
+        case VIR_DOMAIN_MEMORY_MODEL_NONE:
+        case VIR_DOMAIN_MEMORY_MODEL_LAST:
+            break;
+        }
+    }
+
+    if (virBufferUse(&epcBuf) > 0) {
+        virCommandAddArg(cmd, "-M");
+        virCommandAddArgBuffer(cmd, &epcBuf);
     }
 
     return 0;
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..e1aa274054
--- /dev/null
+++ b/tests/qemuxml2argvdata/sgx-epc.x86_64-6.2.0.args
@@ -0,0 +1,38 @@
+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 \
+-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 \
+-object '{"qom-type":"memory-backend-epc","id":"memepc0","prealloc":true,"size":67108864}' \
+-object '{"qom-type":"memory-backend-epc","id":"memepc1","prealloc":true,"size":16777216}' \
+-M sgx-epc.0.memdev=memepc0,sgx-epc.1.memdev=memepc1 \
+-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 \
+-audiodev '{"id":"audio1","driver":"none"}' \
+-device 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/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 96d30f2475..2df18e4199 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -3521,6 +3521,8 @@ mymain(void)
     /* HVF guests should not work on Linux with KVM */
     DO_TEST_CAPS_LATEST_PARSE_ERROR("hvf-x86_64-q35-headless");
 
+    DO_TEST_CAPS_VER("sgx-epc", "6.2.0");
+
     if (getenv("LIBVIRT_SKIP_CLEANUP") == NULL)
         virFileDeleteTree(fakerootdir);
 
-- 
2.17.1

On 2/8/22 06:21, Haibin Huang wrote:
> From: Lin Yang <lin.a.yang@intel.com>
> 
> 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@intel.com>
> ---
>  src/qemu/qemu_alias.c                         |  3 +-
>  src/qemu/qemu_command.c                       | 46 +++++++++++++++++--
>  .../sgx-epc.x86_64-6.2.0.args                 | 38 +++++++++++++++
>  tests/qemuxml2argvtest.c                      |  2 +
>  4 files changed, 84 insertions(+), 5 deletions(-)
>  create mode 100644 tests/qemuxml2argvdata/sgx-epc.x86_64-6.2.0.args
> 
> diff --git a/src/qemu/qemu_alias.c b/src/qemu/qemu_alias.c
> index e5a946cbed..03c79bcf0e 100644
> --- a/src/qemu/qemu_alias.c
> +++ b/src/qemu/qemu_alias.c
> @@ -467,7 +467,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 e6f29d878c..71804e659c 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -3815,6 +3815,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 if (useHugepage || mem->nvdimmPath || memAccess ||
>          def->mem.source == VIR_DOMAIN_MEMORY_SOURCE_FILE) {
>  
> @@ -3972,6 +3976,12 @@ qemuBuildMemoryBackendProps(virJSONValue **backendProps,
>                             _("this qemu doesn't support the "
>                               "memory-backend-memfd object"));
>              return -1;
> +        } else if (STREQ(backendType, "memory-backend-epc") &&
> +                   !virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_SGX_EPC)) {
> +            virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
> +                           _("this qemu doesn't support the "
> +                             "memory-backend-epc object"));

Please put error messages onto a single line. They are extempt from the
80 chars rule. I know you just tried to mimic pre-existing code, but
that is wrong too.

> +            return -1;
>          }
>  
>          rc = 0;
> @@ -7800,6 +7810,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 */
> @@ -7809,11 +7821,37 @@ qemuBuildMemoryDeviceCommandLine(virCommand *cmd,
>          if (qemuBuildMemoryDimmBackendStr(cmd, def->mems[i], def, cfg, priv) < 0)
>              return -1;
>  
> -        if (!(props = qemuBuildMemoryDeviceProps(cfg, priv, 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(cfg, priv, 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=mem%s", epcNum++,
> +                              def->mems[i]->info.alias);
> +
> +            break;
> +
> +        case VIR_DOMAIN_MEMORY_MODEL_NONE:
> +        case VIR_DOMAIN_MEMORY_MODEL_LAST:
> +            break;
> +        }
> +    }
> +
> +    if (virBufferUse(&epcBuf) > 0) {
> +        virCommandAddArg(cmd, "-M");
> +        virCommandAddArgBuffer(cmd, &epcBuf);
>      }
>  
>      return 0;
> 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..e1aa274054
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/sgx-epc.x86_64-6.2.0.args
> @@ -0,0 +1,38 @@
> +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 \
> +-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 \
> +-object '{"qom-type":"memory-backend-epc","id":"memepc0","prealloc":true,"size":67108864}' \
> +-object '{"qom-type":"memory-backend-epc","id":"memepc1","prealloc":true,"size":16777216}' \
> +-M sgx-epc.0.memdev=memepc0,sgx-epc.1.memdev=memepc1 \

I don't think this is correct. IIUC, this can be passed to -machine
directly, e.g.:

-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

And when I try to do that, I get:

  qemu-system-x86_64: Parameter 'sgx-epc.0.node' is missing

Any idea, what's going on? I would much rather avoid using -M if we can
help it.

Michal

On 2/16/22 11:25, Michal Prívozník wrote:

>> 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..e1aa274054
>> --- /dev/null
>> +++ b/tests/qemuxml2argvdata/sgx-epc.x86_64-6.2.0.args
>> @@ -0,0 +1,38 @@
>> +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 \
>> +-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 \
>> +-object '{"qom-type":"memory-backend-epc","id":"memepc0","prealloc":true,"size":67108864}' \
>> +-object '{"qom-type":"memory-backend-epc","id":"memepc1","prealloc":true,"size":16777216}' \
>> +-M sgx-epc.0.memdev=memepc0,sgx-epc.1.memdev=memepc1 \
> 
> I don't think this is correct. IIUC, this can be passed to -machine
> directly, e.g.:
> 
> -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
> 
> And when I try to do that, I get:
> 
>   qemu-system-x86_64: Parameter 'sgx-epc.0.node' is missing
> 
> Any idea, what's going on? I would much rather avoid using -M if we can
> help it.
> 

One more thing. I've enabled SGX on my machine hoping to test this out,
but my attempts are failing so far. Firstly, with these patches qemu is
unhappy, because .node attribute is missing:

/home/zippy/work/qemu/qemu.git/build/qemu-system-x86_64 \

-machine pc-i440fx-4.0,usb=off,dump-guest-core=off \
-accel kvm \
-cpu host,migratable=on \
-m size=4194304k,slots=16,maxmem=1099511627776k \

-object '{"qom-type":"memory-backend-memfd","id":"memepc0","hugetlb":true,"hugetlbsize":2097152,"prealloc":true,"size":67108864,"host-nodes":[0],"policy":"bind"}' \
-M sgx-epc.0.memdev=memepc0 \

qemu-system-x86_64: Parameter 'sgx-epc.0.node' is missing


But okay, I can add .node, but that doesn't get me much further:

/home/zippy/work/qemu/qemu.git/build/qemu-system-x86_64 \

-machine pc-i440fx-4.0,usb=off,dump-guest-core=off \
-accel kvm \
-cpu host,migratable=on \
-m size=4194304k,slots=16,maxmem=1099511627776k \

-object '{"qom-type":"memory-backend-memfd","id":"memepc0","hugetlb":true,"hugetlbsize":2097152,"prealloc":true,"size":67108864,"host-nodes":[0],"policy":"bind"}' \
-M sgx-epc.0.memdev=memepc0,sgx-epc.0.node=0 \

qemu-system-x86_64: Invalid parameter type for 'memdev', expected: memory-backend-epc


Michal

On 2/22/22, 1:19 AM, "Michal Prívozník" <mprivozn@redhat.com> wrote:

> On 2/16/22 11:25, Michal Prívozník wrote:
>
> >> 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..e1aa274054
> >> --- /dev/null
> >> +++ b/tests/qemuxml2argvdata/sgx-epc.x86_64-6.2.0.args
> >> @@ -0,0 +1,38 @@
> >> +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 \
> >> +-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 \
> >> +-object '{"qom-type":"memory-backend-epc","id":"memepc0","prealloc":true,"size":67108864}' \
> >> +-object '{"qom-type":"memory-backend-epc","id":"memepc1","prealloc":true,"size":16777216}' \
> >> +-M sgx-epc.0.memdev=memepc0,sgx-epc.1.memdev=memepc1 \
> >
> > I don't think this is correct. IIUC, this can be passed to -machine
> > directly, e.g.:
> >
> > -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
> >
> > And when I try to do that, I get:
> >
> >   qemu-system-x86_64: Parameter 'sgx-epc.0.node' is missing
> >
> > Any idea, what's going on? I would much rather avoid using -M if we can
> > help it.
> >
>
> One more thing. I've enabled SGX on my machine hoping to test this out,
> but my attempts are failing so far. Firstly, with these patches qemu is
> unhappy, because .node attribute is missing:
>
> /home/zippy/work/qemu/qemu.git/build/qemu-system-x86_64 \
>
> -machine pc-i440fx-4.0,usb=off,dump-guest-core=off \
> -accel kvm \
> -cpu host,migratable=on \
> -m size=4194304k,slots=16,maxmem=1099511627776k \
>
> -object '{"qom-type":"memory-backend-memfd","id":"memepc0","hugetlb":true,"hugetlbsize":2097152,"prealloc":true,"size":67108864,"host-nodes":[0],"policy":"bind"}' \
> -M sgx-epc.0.memdev=memepc0 \
>
> qemu-system-x86_64: Parameter 'sgx-epc.0.node' is missing

Did you use qemu latest master branch here for experiments? NUMA
support is added in qemu master branch, which requires 'sgx-epc.0.node'
parameter, but not in its latest 6.2.0 release.

So here I didn’t add 'sgx-epc.0.node' parameter to align with qemu 6.2.0.
NUMA support could be separate patch after qemu new release. The other
options is we add NUMA support at this time. Any suggestions?

>
> But okay, I can add .node, but that doesn't get me much further:
>
> /home/zippy/work/qemu/qemu.git/build/qemu-system-x86_64 \
>
> -machine pc-i440fx-4.0,usb=off,dump-guest-core=off \
> -accel kvm \
> -cpu host,migratable=on \
> -m size=4194304k,slots=16,maxmem=1099511627776k \
>
> -object '{"qom-type":"memory-backend-memfd","id":"memepc0","hugetlb":true,"hugetlbsize":2097152,"prealloc":true,"size":67108864,"host-nodes":[0],"policy":"bind"}' \
> -M sgx-epc.0.memdev=memepc0,sgx-epc.0.node=0 \
>
> qemu-system-x86_64: Invalid parameter type for 'memdev', expected: memory-backend-epc

I think I fixed this kind of issue by moving memory-backend-epc type check before hugetlb
check. Could you pls share your domain xml, so I can reproduce it on my side? Thanks.

Thanks,
Lin.

On 2/24/22 21:19, Yang, Lin A wrote:
> On 2/22/22, 1:19 AM, "Michal Prívozník" <mprivozn@redhat.com> wrote:
> 
>  
> 
>> On 2/16/22 11:25, Michal Prívozník wrote:
> 
>> 
> 
>> >> 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..e1aa274054
> 
>> >> --- /dev/null
> 
>> >> +++ b/tests/qemuxml2argvdata/sgx-epc.x86_64-6.2.0.args
> 
>> >> @@ -0,0 +1,38 @@
> 
>> >> +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 \
> 
>> >> +-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 \
> 
>> >> +-object '{"qom-type":"memory-backend-epc","id":"memepc0","prealloc":true,"size":67108864}' \
> 
>> >> +-object '{"qom-type":"memory-backend-epc","id":"memepc1","prealloc":true,"size":16777216}' \
> 
>> >> +-M sgx-epc.0.memdev=memepc0,sgx-epc.1.memdev=memepc1 \
> 
>> > 
> 
>> > I don't think this is correct. IIUC, this can be passed to -machine
> 
>> > directly, e.g.:
> 
>> > 
> 
>> > -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
> 
>> > 
> 
>> > And when I try to do that, I get:
> 
>> > 
> 
>> >   qemu-system-x86_64: Parameter 'sgx-epc.0.node' is missing
> 
>> > 
> 
>> > Any idea, what's going on? I would much rather avoid using -M if we can
> 
>> > help it.
> 
>> > 
> 
>> 
> 
>> One more thing. I've enabled SGX on my machine hoping to test this out,
> 
>> but my attempts are failing so far. Firstly, with these patches qemu is
> 
>> unhappy, because .node attribute is missing:
> 
>> 
> 
>> /home/zippy/work/qemu/qemu.git/build/qemu-system-x86_64 \
> 
>> 
> 
>> -machine pc-i440fx-4.0,usb=off,dump-guest-core=off \
> 
>> -accel kvm \
> 
>> -cpu host,migratable=on \
> 
>> -m size=4194304k,slots=16,maxmem=1099511627776k \
> 
>> 
> 
>> -object '{"qom-type":"memory-backend-memfd","id":"memepc0","hugetlb":true,"hugetlbsize":2097152,"prealloc":true,"size":67108864,"host-nodes":[0],"policy":"bind"}' \
> 
>> -M sgx-epc.0.memdev=memepc0 \
> 
>> 
> 
>> qemu-system-x86_64: Parameter 'sgx-epc.0.node' is missing
> 
>  
> 
> Did you use qemu latest master branch here for experiments? NUMA
> 
> support is added in qemu master branch, which requires 'sgx-epc.0.node'
> 
> parameter, but not in its latest 6.2.0 release.

Yes, I'm using:


qemu.git $ git describe 
v6.2.0-1718-g922268067f

> 
>  
> 
> So here I didn’t add 'sgx-epc.0.node' parameter to align with qemu 6.2.0.
> 
> NUMA support could be separate patch after qemu new release. The other
> 
> options is we add NUMA support at this time. Any suggestions?

Since the argument is going to be required I think it makes sense to
require it for qemu 6.2.0. Otherwise we could have yet another
capability to reflect whether .node attribute is needed or not. But IMO
that's pointless because the capability would be set only for one
particular release of QEMU (not to mention it's probably impossible to
detect whether .node attribute is needed or not).

> 
>  
> 
>> 
> 
>> But okay, I can add .node, but that doesn't get me much further:
> 
>> 
> 
>> /home/zippy/work/qemu/qemu.git/build/qemu-system-x86_64 \
> 
>> 
> 
>> -machine pc-i440fx-4.0,usb=off,dump-guest-core=off \
> 
>> -accel kvm \
> 
>> -cpu host,migratable=on \
> 
>> -m size=4194304k,slots=16,maxmem=1099511627776k \
> 
>> 
> 
>> -object '{"qom-type":"memory-backend-memfd","id":"memepc0","hugetlb":true,"hugetlbsize":2097152,"prealloc":true,"size":67108864,"host-nodes":[0],"policy":"bind"}' \
> 
>> -M sgx-epc.0.memdev=memepc0,sgx-epc.0.node=0 \
> 
>> 
> 
>> qemu-system-x86_64: Invalid parameter type for 'memdev', expected: memory-backend-epc
> 
>  
> 
> I think I fixed this kind of issue by moving memory-backend-epc type
> check before hugetlb
> 
> check. Could you pls share your domain xml, so I can reproduce it on my
> side? Thanks.


Sure:

<domain type='kvm'>
  <name>fedora</name>
  <uuid>63840878-0deb-4095-97e6-fc444d9bc9fa</uuid>
  <title>hello</title>
  <description>blee</description>
  <maxMemory slots='16' unit='KiB'>1099511627776</maxMemory>
  <memory unit='KiB'>4210688</memory>
  <currentMemory unit='KiB'>4194304</currentMemory>
  <memoryBacking>
    <hugepages>
      <page size='2048' unit='KiB'/>
    </hugepages>
    <source type='memfd'/>
    <allocation mode='immediate'/>
  </memoryBacking>
  <vcpu placement='static'>4</vcpu>
  <iothreads>4</iothreads>
  <cputune>
    <vcpupin vcpu='0' cpuset='0-1'/>
    <vcpupin vcpu='1' cpuset='2-3'/>
    <vcpupin vcpu='2' cpuset='4-5'/>
    <vcpupin vcpu='3' cpuset='6-7'/>
  </cputune>
  <numatune>
    <memory mode='strict' nodeset='0'/>
  </numatune>
  <os>
    <type arch='x86_64' machine='pc-i440fx-7.0'>hvm</type>
    <bootmenu enable='yes'/>
  </os>
  <features>
    <acpi/>
    <apic/>
    <pae/>
  </features>
  <cpu mode='host-passthrough' check='none' migratable='on'>
    <topology sockets='1' dies='1' cores='2' threads='2'/>
    <numa>
      <cell id='0' cpus='0,2' memory='2097152' unit='KiB' discard='yes'/>
      <cell id='1' cpus='1,3' memory='2097152' unit='KiB' discard='yes'/>
    </numa>
  </cpu>
  <clock offset='utc'/>
  <on_poweroff>destroy</on_poweroff>
  <on_reboot>restart</on_reboot>
  <on_crash>restart</on_crash>
  <pm>
    <suspend-to-mem enabled='yes'/>
    <suspend-to-disk enabled='yes'/>
  </pm>
  <devices>
    <emulator>/home/zippy/work/qemu/qemu.git/build/qemu-system-x86_64</emulator>
    <disk type='file' device='disk'>
      <driver name='qemu' type='qcow2' discard='unmap'/>
      <source file='/var/lib/libvirt/images/fedora.qcow2'/>
      <target dev='sda' bus='scsi'/>
      <boot order='1'/>
      <address type='drive' controller='0' bus='0' target='0' unit='0'/>
    </disk>
    <disk type='file' device='cdrom'>
      <driver name='qemu' type='raw'/>
      <target dev='sdb' bus='sata'/>
      <readonly/>
      <address type='drive' controller='0' bus='0' target='0' unit='1'/>
    </disk>
    <controller type='pci' index='0' model='pci-root'/>
    <controller type='scsi' index='0' model='virtio-scsi'>
      <address type='pci' domain='0x0000' bus='0x00' slot='0x04' function='0x0'/>
    </controller>
    <controller type='virtio-serial' index='0'>
      <address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x0'/>
    </controller>
    <controller type='usb' index='0' model='ich9-ehci1'>
      <address type='pci' domain='0x0000' bus='0x00' slot='0x08' function='0x7'/>
    </controller>
    <controller type='usb' index='0' model='ich9-uhci1'>
      <master startport='0'/>
      <address type='pci' domain='0x0000' bus='0x00' slot='0x08' function='0x0' multifunction='on'/>
    </controller>
    <controller type='usb' index='0' model='ich9-uhci2'>
      <master startport='2'/>
      <address type='pci' domain='0x0000' bus='0x00' slot='0x08' function='0x1'/>
    </controller>
    <controller type='usb' index='0' model='ich9-uhci3'>
      <master startport='4'/>
      <address type='pci' domain='0x0000' bus='0x00' slot='0x08' function='0x2'/>
    </controller>
    <controller type='sata' index='0'>
      <address type='pci' domain='0x0000' bus='0x00' slot='0x09' function='0x0'/>
    </controller>
    <interface type='network' trustGuestRxFilters='no'>
      <mac address='52:54:00:a4:6f:91'/>
      <source network='default'/>
      <bandwidth>
        <inbound average='1024' peak='4096' floor='500' burst='2048'/>
        <outbound average='10240' burst='2048'/>
      </bandwidth>
      <model type='virtio'/>
      <mtu size='9000'/>
      <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0'/>
    </interface>
    <serial type='pty'>
      <target type='isa-serial' port='0'>
        <model name='isa-serial'/>
      </target>
    </serial>
    <console type='pty'>
      <target type='serial' port='0'/>
    </console>
    <channel type='unix'>
      <target type='virtio' name='org.qemu.guest_agent.0'/>
      <address type='virtio-serial' controller='0' bus='0' port='1'/>
    </channel>
    <input type='mouse' bus='ps2'/>
    <input type='keyboard' bus='ps2'/>
    <graphics type='spice' autoport='yes'>
      <listen type='address'/>
      <gl enable='no'/>
    </graphics>
    <audio id='1' type='spice'/>
    <video>
      <model type='virtio' heads='1' primary='yes'>
        <acceleration accel3d='no'/>
      </model>
      <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/>
    </video>
    <memballoon model='virtio'>
      <address type='pci' domain='0x0000' bus='0x00' slot='0x06' function='0x0'/>
    </memballoon>
    <memory model='sgx-epc'>
      <target>
        <size unit='KiB'>16384</size>
      </target>
    </memory>
  </devices>
</domain>


> 
>  
> 
> Thanks,
> 
> Lin.
> 

On 2/25/22, 5:48 AM, "Michal Prívozník" <mprivozn@redhat.com> wrote:

> Since the argument is going to be required I think it makes sense to
> require it for qemu 6.2.0. Otherwise we could have yet another
> capability to reflect whether .node attribute is needed or not. But IMO
> that's pointless because the capability would be set only for one
> particular release of QEMU (not to mention it's probably impossible to
> detect whether .node attribute is needed or not).

Really appreciate your comments!

We are re-designing our solution to support NUMA case, and might
also need to refactor current code because of under reviewing SGX
QEMU patch. Will push for reviewing once all of them are done.

Thanks,
Lin.