On Wed, Jul 27, 2022 at 12:34:57 +0200, Michal Privoznik wrote:
> From: Lin Yang <lin.a.yang@xxxxxxxxx>
>
> 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@xxxxxxxxx>
> Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
> ---
[...]
> diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
> index 1ed969ac3e..bdd0fcea8e 100644
> --- a/docs/formatdomain.rst
> +++ b/docs/formatdomain.rst
> @@ -7940,6 +7940,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>
> ...
>
> @@ -7948,7 +7962,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.7.0 and QEMU 6.2.0`
I don't quite understand from this description whether this is real
memory usable by the guest OS or something for internal use by the
hypervisor.
Specifically which leads me to questioning this is that the example
sizes are very tiny compared to real memory sizing.
Additionally in qemuDomainDefValidateMemoryHotplug you are changing the
code to specifically exclude the sizing of the 'sgx-epc' memory devices
from the total size of the memory, but this contrasts with the memory
_not_ being excluded from the initial memory calculation in
virDomainDefGetMemoryInitial which is used to format the initial memory
argument ('-m size=...'). Thus at least one of them is wrong.
If this is not guest usable memory, then the <memory> element should not
be used to bolt this on, but rather add a new element similarly to what
we have when AMD SEV is in use.
>
> ``access``
> An optional attribute ``access`` ( :since:`since 3.2.0` ) that provides
> @@ -8008,6 +8024,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.7.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.
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index e85cc1f809..a1f64b4fcb 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -1440,6 +1440,7 @@ VIR_ENUM_IMPL(virDomainMemoryModel,
> "nvdimm",
> "virtio-pmem",
> "virtio-mem",
> + "sgx-epc",
> );
>
> VIR_ENUM_IMPL(virDomainShmemModel,
> @@ -13303,6 +13304,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,
A more appropriate error code would be VIR_ERR_XML_DETAIL or VIR_ERR_INVALID_ARG
> + _("Invalid value of 'nodemask': %s"), nodemask);
> + return -1;
> + }
> + }
> + break;
> +
> case VIR_DOMAIN_MEMORY_MODEL_NONE:
> case VIR_DOMAIN_MEMORY_MODEL_LAST:
> break;
> @@ -13371,6 +13386,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;
> }
> @@ -15167,6 +15183,11 @@ virDomainMemoryFindByDefInternal(virDomainDef *def,
> continue;
> break;
>
> + case VIR_DOMAIN_MEMORY_MODEL_SGX_EPC:
> + if (!virBitmapEqual(tmp->sourceNodes, mem->sourceNodes))
> + continue;
> + break;
> +
> case VIR_DOMAIN_MEMORY_MODEL_NONE:
> case VIR_DOMAIN_MEMORY_MODEL_LAST:
> break;
> @@ -24778,6 +24799,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/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
> index 764d5b029e..259636f7e7 100644
> --- a/src/qemu/qemu_validate.c
> +++ b/src/qemu/qemu_validate.c
> @@ -5181,6 +5181,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 9f2872decc..98044d1847 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..9029977f20
> --- /dev/null
> +++ b/tests/qemuxml2argvdata/sgx-epc-numa.xml
> @@ -0,0 +1,64 @@
> +<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>
> + <os>
> + <type arch='x86_64' machine='q35'>hvm</type>
> + <boot dev='hd'/>
> + </os>
> + <cpu mode='custom' match='exact' check='none'>
> + <model fallback='forbid'>qemu64</model>
> + <numa>
> + <cell id='0' cpus='0' memory='109550' unit='KiB'/>
> + <cell id='1' cpus='1' memory='109550' unit='KiB'/>
> + </numa>
> + </cpu>
> + <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='pci' index='1' model='pcie-root-port'>
> + <model name='pcie-root-port'/>
> + <target chassis='1' port='0x8'/>
> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0' multifunction='on'/>
> + </controller>
> + <controller type='pci' index='2' model='pcie-root-port'>
> + <model name='pcie-root-port'/>
> + <target chassis='2' port='0x9'/>
> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x1'/>
> + </controller>
> + <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'/>
> + <memballoon model='virtio'>
> + <address type='pci' domain='0x0000' bus='0x01' slot='0x00' function='0x0'/>
> + </memballoon>
> + <memory model='sgx-epc'>
> + <source>
> + <nodemask>0-1</nodemask>
> + </source>
> + <target>
> + <size unit='KiB'>65536</size>
> + <node>0</node>
> + </target>
> + </memory>
> + <memory model='sgx-epc'>
> + <source>
> + <nodemask>2-3</nodemask>
> + </source>
> + <target>
> + <size unit='KiB'>16384</size>
> + <node>1</node>
> + </target>
> + </memory>
> + </devices>
> +</domain>
> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c
> index 4cbf380e44..033efad646 100644
> --- a/tests/qemuxml2xmltest.c
> +++ b/tests/qemuxml2xmltest.c
> @@ -1462,6 +1462,9 @@ mymain(void)
> DO_TEST_CAPS_LATEST("channel-qemu-vdagent");
> DO_TEST_CAPS_LATEST("channel-qemu-vdagent-features");
>
> + DO_TEST_CAPS_VER("sgx-epc", "6.2.0");
> + DO_TEST_CAPS_LATEST("sgx-epc-numa");
This test will break once I re-generate latest caps. Please pin it to
qemu-7.0.
