On Wed, Jul 27, 2022 at 12:34:56 +0200, Michal Privoznik wrote:
> From: Haibin Huang <haibin.huang@xxxxxxxxx>
>
> Extend hypervisor capabilities to include sgx feature. When available,
> the hypervisor supports launching an VM with SGX on Intel platfrom.
> The SGX feature tag privides additional details like section size and
> sgx1 or sgx2.
>
> Signed-off-by: Haibin Huang <haibin.huang@xxxxxxxxx>
> Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
> ---
[...]
>
> diff --git a/docs/formatdomaincaps.rst b/docs/formatdomaincaps.rst
> index 70f46b972a..bca5389931 100644
> --- a/docs/formatdomaincaps.rst
> +++ b/docs/formatdomaincaps.rst
> @@ -554,6 +554,16 @@ capabilities. All features occur as children of the main ``features`` element.
> <cbitpos>47</cbitpos>
> <reduced-phys-bits>1</reduced-phys-bits>
> </sev>
> + <sgx supported='yes'>
> + <flc>no</flc>
> + <sgx1>yes</sgx1>
> + <sgx2>no</sgx2>
> + <section_size>2</section_size>
This element has "unit='KiB'" in the output data in the tests.
> + <sections>
> + <section node='0' size='1'/>
> + <section node='1' size='1'/>
And this one should get a unit too, since the above has it.
> + </sections>
> + </sgx>
> </features>
> </domainCapabilities>
>
> @@ -633,3 +643,33 @@ in domain XML <formatdomain.html#launch-security>`__
> ``maxESGuests``
> The maximum number of SEV-ES guests that can be launched on the host. This
> value may be configurable in the firmware for some hosts.
> +
> +SGX capabilities
> +^^^^^^^^^^^^^^^^
> +
> +Intel Software Guard Extensions (Intel SGX) capabilities are exposed under the
> +``sgx`` element.
> +
> +Intel SGX helps protect data in use via unique application isolation technology.
> +Protect selected code and data from modification using hardened enclaves with
> +Intel SGX.
> +
> +For more details on the SGX feature, please follow resources in the SGX developer's
> +document store. In order to use SGX with libvirt have a look at formatdomain.rst
> +Memory devices.
This should be a real RST link to the document/section you are refering
to.
> +
> +``flc``
> + FLC (Flexible Launch Control), not strictly part of SGX2, but was not part of
> + original SGX hardware either.
> +
> +``sgx1``
> + the sgx version 1.
> +
> +``sgx2``
> + The sgx version 2.
> +
> +``section_size``
> + The size of the SGX enclave page cache (called EPC).
> +
> +``sections``
> + The sections of the SGX enclave page cache (called EPC).
> diff --git a/src/conf/domain_capabilities.c b/src/conf/domain_capabilities.c
> index 1d3bba3019..ef41612494 100644
> --- a/src/conf/domain_capabilities.c
> +++ b/src/conf/domain_capabilities.c
[...]
> @@ -621,6 +622,39 @@ virDomainCapsFeatureSEVFormat(virBuffer *buf,
> virBufferAddLit(buf, "</sev>\n");
> }
>
> +static void
> +virDomainCapsFeatureSGXFormat(virBuffer *buf,
> + const virSGXCapability *sgx)
> +{
> + if (!sgx) {
> + virBufferAddLit(buf, "<sgx supported='no'/>\n");
> + return;
> + }
> +
> + virBufferAddLit(buf, "<sgx supported='yes'>\n");
> + virBufferAdjustIndent(buf, 2);
> + virBufferAsprintf(buf, "<flc>%s</flc>\n", sgx->flc ? "yes" : "no");
> + virBufferAsprintf(buf, "<sgx1>%s</sgx1>\n", sgx->sgx1 ? "yes" : "no");
> + virBufferAsprintf(buf, "<sgx2>%s</sgx2>\n", sgx->sgx2 ? "yes" : "no");
> + virBufferAsprintf(buf, "<section_size unit='KiB'>%llu</section_size>\n", sgx->section_size);
> +
> + if (sgx->nsections > 0) {
> + size_t i;
> +
> + virBufferAddLit(buf, "<sections>\n");
> +
> + for (i = 0; i < sgx->nsections; i++) {
> + virBufferAdjustIndent(buf, 2);
> + virBufferAsprintf(buf, "<section node='%d' ", sgx->sections[i].node);
> + virBufferAsprintf(buf, "size='%llu'/>\n", sgx->sections[i].size);
I think you want to ad a unit field too here.
> + virBufferAdjustIndent(buf, -2);
> + }
> + virBufferAddLit(buf, "</sections>\n");
> + }
> +
> + virBufferAdjustIndent(buf, -2);
> + virBufferAddLit(buf, "</sgx>\n");
This looks almost identical to the formatter for the qemu capability
cache. Would it make sense to factor it out and reuse it instead of
reimplementing?
[...]
> diff --git a/tests/domaincapsdata/qemu_7.0.0-q35.x86_64.xml b/tests/domaincapsdata/qemu_7.0.0-q35.x86_64.xml
> index 4e6ff06125..5eff1a0ff9 100644
> --- a/tests/domaincapsdata/qemu_7.0.0-q35.x86_64.xml
> +++ b/tests/domaincapsdata/qemu_7.0.0-q35.x86_64.xml
> @@ -228,5 +228,15 @@
> <backingStoreInput supported='yes'/>
> <backup supported='yes'/>
> <sev supported='no'/>
> + <sgx supported='yes'>
> + <flc>no</flc>
> + <sgx1>yes</sgx1>
> + <sgx2>no</sgx2>
> + <section_size unit='KiB'>2</section_size>
> + <sections>
> + <section node='0' size='1'/>
> + <section node='1' size='1'/>
This looks weird without unit.
> + </sections>
> + </sgx>
> </features>
> </domainCapabilities>
