Re: [PATCH RESEND v12 5/6] conf: Introduce SGX EPC element into device memory xml

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 5/18/22 09:59, 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>
> Signed-off-by: Haibin Huang <haibin.huang@xxxxxxxxx>
> ---
>  docs/formatdomain.rst                         |  9 +++-
>  src/conf/domain_conf.c                        |  6 +++
>  src/conf/domain_conf.h                        |  1 +
>  src/conf/domain_validate.c                    | 16 ++++++
>  src/conf/schemas/domaincommon.rng             |  1 +
>  src/qemu/qemu_alias.c                         |  3 ++
>  src/qemu/qemu_command.c                       |  1 +
>  src/qemu/qemu_domain.c                        | 38 +++++++++-----
>  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 +
>  tests/qemuxml2argvdata/sgx-epc.xml            | 36 +++++++++++++
>  .../sgx-epc.x86_64-6.2.0.xml                  | 52 +++++++++++++++++++
>  tests/qemuxml2xmltest.c                       |  2 +
>  18 files changed, 172 insertions(+), 15 deletions(-)
>  create mode 100644 tests/qemuxml2argvdata/sgx-epc.xml
>  create mode 100644 tests/qemuxml2xmloutdata/sgx-epc.x86_64-6.2.0.xml
> 
> diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
> index c1e99951a6..20aee6c639 100644
> --- a/docs/formatdomain.rst
> +++ b/docs/formatdomain.rst
> @@ -7859,6 +7859,11 @@ Example: usage of the memory devices
>           <current unit='KiB'>524288</current>
>         </target>
>       </memory>
> +     <memory model='sgx-epc'>
> +       <target>
> +         <size unit='KiB'>16384</size>
> +       </target>
> +     </memory>
>     </devices>
>     ...
>  
> @@ -7867,7 +7872,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.4.0 and QEMU 6.2.0`
>  
>  ``access``
>     An optional attribute ``access`` ( :since:`since 3.2.0` ) that provides
> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> index 70562cc993..ebd919083d 100644
> --- a/src/conf/domain_conf.c
> +++ b/src/conf/domain_conf.c
> @@ -1430,6 +1430,7 @@ VIR_ENUM_IMPL(virDomainMemoryModel,
>                "nvdimm",
>                "virtio-pmem",
>                "virtio-mem",
> +              "sgx-epc",
>  );
>  
>  VIR_ENUM_IMPL(virDomainShmemModel,
> @@ -5641,6 +5642,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;
> @@ -14596,6 +14598,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;
> @@ -14664,6 +14667,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;
>      }
> @@ -16460,6 +16464,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;
> @@ -25935,6 +25940,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 2e2da0c69c..8bbaacde38 100644
> --- a/src/conf/domain_conf.h
> +++ b/src/conf/domain_conf.h
> @@ -2540,6 +2540,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 18eb8d697d..e25d5b663c 100644
> --- a/src/conf/domain_validate.c
> +++ b/src/conf/domain_validate.c
> @@ -2159,6 +2159,22 @@ virDomainMemoryDefValidate(const virDomainMemoryDef *mem,
>      case VIR_DOMAIN_MEMORY_MODEL_DIMM:
>          break;
>  
> +    case VIR_DOMAIN_MEMORY_MODEL_SGX_EPC:
> +        if (mem->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) {
> +            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                           _("memory device address is not supported for model '%s'"),
> +                           virDomainMemoryModelTypeToString(mem->model));
> +            return -1;
> +        }
> +
> +        if (mem->targetNode != -1) {
> +            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                           _("NUMA nodes is not supported for model '%s'"),
> +                           virDomainMemoryModelTypeToString(mem->model));
> +            return -1;
> +        }
> +        break;
> +
>      case VIR_DOMAIN_MEMORY_MODEL_NONE:
>      case VIR_DOMAIN_MEMORY_MODEL_LAST:
>      default:
> diff --git a/src/conf/schemas/domaincommon.rng b/src/conf/schemas/domaincommon.rng
> index 2544864eb4..f00d3692f7 100644
> --- a/src/conf/schemas/domaincommon.rng
> +++ b/src/conf/schemas/domaincommon.rng
> @@ -6718,6 +6718,7 @@
>            <value>nvdimm</value>
>            <value>virtio-pmem</value>
>            <value>virtio-mem</value>
> +          <value>sgx-epc</value>
>          </choice>
>        </attribute>
>        <optional>
> diff --git a/src/qemu/qemu_alias.c b/src/qemu/qemu_alias.c
> index 8c2f055604..e5a946cbed 100644
> --- a/src/qemu/qemu_alias.c
> +++ b/src/qemu/qemu_alias.c
> @@ -516,6 +516,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 a52ba70066..4807b137b6 100644
> --- a/src/qemu/qemu_command.c
> +++ b/src/qemu/qemu_command.c
> @@ -4041,6 +4041,7 @@ qemuBuildMemoryDeviceProps(virQEMUDriverConfig *cfg,
>              return NULL;
>          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 4305d5db06..2adf346caa 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -8438,6 +8438,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;
> @@ -9119,6 +9120,12 @@ qemuDomainDefValidateMemoryHotplugDevice(const virDomainMemoryDef *mem,
>          }
>          break;
>  
> +    case VIR_DOMAIN_MEMORY_MODEL_SGX_EPC:
> +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                       _("hotplug are not supported for the %s device"),
> +                       virDomainMemoryModelTypeToString(mem->model));
> +            return -1;
> +
>      case VIR_DOMAIN_MEMORY_MODEL_NONE:
>      case VIR_DOMAIN_MEMORY_MODEL_LAST:
>          return -1;
> @@ -9154,7 +9161,7 @@ int
>  qemuDomainDefValidateMemoryHotplug(const virDomainDef *def,
>                                     const virDomainMemoryDef *mem)
>  {
> -    unsigned int nmems = def->nmems;
> +    unsigned int hotplugNum = 0;
>      unsigned long long hotplugSpace;
>      unsigned long long hotplugMemory = 0;
>      size_t i;
> @@ -9162,15 +9169,27 @@ qemuDomainDefValidateMemoryHotplug(const virDomainDef *def,
>      hotplugSpace = def->mem.max_memory - virDomainDefGetMemoryInitial(def);
>  
>      if (mem) {
> -        nmems++;
> +        hotplugNum++;
>          hotplugMemory = mem->size;
>  
>          if (qemuDomainDefValidateMemoryHotplugDevice(mem, def) < 0)
>              return -1;
>      }
>  
> +    for (i = 0; i < def->nmems; i++) {
> +        /* sgx epc memory does not support hotplug */
> +        if (def->mems[i]->model != VIR_DOMAIN_MEMORY_MODEL_SGX_EPC) {

This is very, very easy to overlook. I mean the condition. Also, notice
how when you added new memory type compiler identified all the places
you need to adjust? That's because I've rewrote the code to use switch()
everywhere. And this new code goes against that direction.

> +            hotplugMemory += def->mems[i]->size;
> +            hotplugNum++;
> +            /* already existing devices don't need to be checked on hotplug */
> +            if (!mem &&
> +                qemuDomainDefValidateMemoryHotplugDevice(def->mems[i], def) < 0)
> +                return -1;
> +        }
> +    }

Michal




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux