On 12/15/21 04:40, Haibin Huang wrote:
> From: Lin Yang <lin.a.yang@xxxxxxxxx>
>
> <devices>
> ...
> <memory model='sgx-epc'>
> <target>
> <size unit='KiB'>512</size>
> </target>
> </memory>
> ...
> </devices>
>
> Signed-off-by: Lin Yang <lin.a.yang@xxxxxxxxx>
> ---
> docs/schemas/domaincommon.rng | 1 +
> src/conf/domain_conf.c | 6 ++++++
> src/conf/domain_conf.h | 1 +
> src/conf/domain_validate.c | 1 +
> src/qemu/qemu_alias.c | 3 +++
> src/qemu/qemu_command.c | 1 +
> src/qemu/qemu_domain.c | 2 ++
> 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 ++
> 14 files changed, 37 insertions(+)
Any domain XML change/extention has to be coupled with documentation
(docs/formatdomain.rst). How would an user know there's a new memory
model and what does its XML look like?
>
> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
> index 26990c4d6d..39b02d1cb7 100644
> --- a/docs/schemas/domaincommon.rng
> +++ b/docs/schemas/domaincommon.rng
> @@ -6616,6 +6616,7 @@
> <value>nvdimm</value>
> <value>virtio-pmem</value>
> <value>virtio-mem</value>
> + <value>sgx-epc</value>
> </choice>
> </attribute>
> <optional>
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 6fcf86ba58..c892865da4 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -1399,6 +1399,7 @@ VIR_ENUM_IMPL(virDomainMemoryModel,
> "nvdimm",
> "virtio-pmem",
> "virtio-mem",
> + "sgx-epc",
> );
>
> VIR_ENUM_IMPL(virDomainShmemModel,
> @@ -5508,6 +5509,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;
> @@ -14696,6 +14698,7 @@ virDomainMemorySourceDefParseXML(xmlNodePtr node,
> def->nvdimmPath = virXPathString("string(./path)", ctxt);
> break;
>
> + case VIR_DOMAIN_MEMORY_MODEL_SGX_EPC:
> case VIR_DOMAIN_MEMORY_MODEL_NONE:
> case VIR_DOMAIN_MEMORY_MODEL_LAST:
> break;
> @@ -14764,6 +14767,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;
> }
> @@ -16548,6 +16552,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;
> @@ -25997,6 +26002,7 @@ virDomainMemorySourceDefFormat(virBuffer *buf,
> virBufferEscapeString(&childBuf, "<path>%s</path>\n", def->nvdimmPath);
> 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/conf/domain_conf.h b/src/conf/domain_conf.h
> index 1ac802feca..58b6ff8355 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -2482,6 +2482,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 80401cf8c7..982ecc60d0 100644
> --- a/src/conf/domain_validate.c
> +++ b/src/conf/domain_validate.c
> @@ -2066,6 +2066,7 @@ virDomainMemoryDefValidate(const virDomainMemoryDef *mem,
> break;
>
> case VIR_DOMAIN_MEMORY_MODEL_DIMM:
> + case VIR_DOMAIN_MEMORY_MODEL_SGX_EPC:
> break;
>
> case VIR_DOMAIN_MEMORY_MODEL_NONE:
> diff --git a/src/qemu/qemu_alias.c b/src/qemu/qemu_alias.c
> index 276a03cb56..5795924754 100644
> --- a/src/qemu/qemu_alias.c
> +++ b/src/qemu/qemu_alias.c
> @@ -538,6 +538,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 dba877a740..36281a69e2 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -3768,6 +3768,7 @@ qemuBuildMemoryDeviceProps(const virDomainDef *def,
> device = "virtio-mem-pci";
> 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 1bd3730281..f156d073e5 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -8184,6 +8184,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;
> @@ -8858,6 +8859,7 @@ qemuDomainDefValidateMemoryHotplugDevice(const virDomainMemoryDef *mem,
> }
> break;
>
> + case VIR_DOMAIN_MEMORY_MODEL_SGX_EPC:
Alright, no hotplug support. But we should report an error here instead
of silently returning -1, because that procudes generic error ("An error
occurred, but the cause is unknown") which is obviously not helping.
> case VIR_DOMAIN_MEMORY_MODEL_NONE:
> case VIR_DOMAIN_MEMORY_MODEL_LAST:
> return -1;
> diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c
> index c43ad23cf5..d9a5e12e8d 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;
> }
> @@ -1025,6 +1026,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;
> }
> @@ -2390,6 +2392,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;
> }
> @@ -3055,6 +3058,7 @@ qemuDomainAssignMemoryDeviceSlot(virQEMUDriver *driver,
> return qemuDomainEnsurePCIAddress(vm, &dev, driver);
> break;
>
> + case VIR_DOMAIN_MEMORY_MODEL_SGX_EPC:
So SGX doesn't live on a PCI bus nor in a DIMM slot. Fair enough, but
then I'd expect a check for that in validator func
(virDomainMemoryDefValidate()). Because what if user provides one?
Now, what I'd like to see here is an qemuxml2xmltest case. At this point
the XML parser/formatter/validator have the support and thus xml2xml
test should pass. At the same time, virschematest will pick up the XML
automatically and test RNG change.
Michal
