RE: [libvirt][PATCH RESEND v10 2/5] conf: expose SGX feature in domain capabilities

-----Original Message----- From: Michal Prívozník <mprivozn(a)redhat.com&gt; Sent: Wednesday, February 16, 2022 6:25 PM To: Huang, Haibin <haibin.huang(a)intel.com>; libvir-list(a)redhat.com; berrange(a)redhat.com; Ding, Jian-feng <jian-feng.ding(a)intel.com>; Yang, Lin A <lin.a.yang(a)intel.com>; Lu, Lianhao <lianhao.lu(a)intel.com&gt; Subject: Re: [libvirt][PATCH RESEND v10 2/5] conf: expose SGX feature in domain capabilities On 2/8/22 06:21, Haibin Huang wrote: > 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(a)intel.com&gt; > --- > docs/formatdomaincaps.html.in | 26 ++++++++++++++++++++++++++ > docs/schemas/domaincaps.rng | 22 +++++++++++++++++++++- > src/conf/domain_capabilities.c | 21 +++++++++++++++++++++ > src/qemu/qemu_capabilities.c | 24 ++++++++++++++++++++++++ > 4 files changed, 92 insertions(+), 1 deletion(-) > > diff --git a/docs/formatdomaincaps.html.in > b/docs/formatdomaincaps.html.in index 35b8bf3def..d932e6df80 100644 > --- a/docs/formatdomaincaps.html.in > +++ b/docs/formatdomaincaps.html.in > @@ -598,6 +598,10 @@ > &lt;cbitpos&gt;47&lt;/cbitpos&gt; > &lt;reduced-phys-bits&gt;1&lt;/reduced-phys-bits&gt; > &lt;/sev&gt; > + &lt;sgx&gt; > + &lt;flc&gt;no&lt;/flc&gt; > + &lt;epc_size&gt;1&lt;/epc_size&gt; > + &lt;/sgx&gt; > &lt;/features&gt; > &lt;/domainCapabilities&gt; > </pre> > @@ -689,5 +693,27 @@ > This value may be configurable in the firmware for some hosts.</dd> > </dl> > > + <h4><a id="elementsSGX">SGX capabilities</a></h4> > + > + <p>Intel Software Guard Extensions (Intel SGX) capabilities are exposed under > + the <code>sgx</code> 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.</p> > + > + <p> > + 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 <a href="formatdomain.html#memory-devices">SGX in domain XML</a> > + </p> > + > + <dl> > + <dt><code>flc</code></dt> > + <dd>FLC (Flexible Launch Control), not strictly part of SGX2, but was not part > + of original SGX hardware either.</dd> > + <dt><code>epc_size</code></dt> > + <dd>The size of the SGX enclave page cache (called EPC).</dd> > + </dl> > + > </body> > </html> > diff --git a/docs/schemas/domaincaps.rng b/docs/schemas/domaincaps.rng > index 9cbc2467ab..5ace30ae0d 100644 > --- a/docs/schemas/domaincaps.rng > +++ b/docs/schemas/domaincaps.rng > @@ -270,6 +270,9 @@ > <optional> > <ref name="sev"/> > </optional> > + <optional> > + <ref name='sgx'/> > + </optional> > </element> > </define> > > @@ -330,7 +333,24 @@ > </element> > </define> > > - <define name="value"> > + <define name='sgx'> > + <element name='sgx'> > + <ref name='supported'/> > + <optional> > + <element name='flc'> > + <data type='string'/> > + </element> > + <element name='epc_size'> > + <attribute name="unit"> > + <value>KiB</value> > + </attribute> > + <data type='unsignedInt'/> > + </element> > + </optional> > + </element> > + </define> > + > + <define name='value'> > <zeroOrMore> > <element name="value"> > <text/> > diff --git a/src/conf/domain_capabilities.c > b/src/conf/domain_capabilities.c index 1170fd26df..2e9f0ec225 100644 > --- a/src/conf/domain_capabilities.c > +++ b/src/conf/domain_capabilities.c > @@ -100,6 +100,7 @@ virDomainCapsDispose(void *obj) > virObjectUnref(caps->cpu.custom); > virCPUDefFree(caps->cpu.hostModel); > virSEVCapabilitiesFree(caps->sev); > + virSGXCapabilitiesFree(caps->sgx); > > values = &caps->os.loader.values; > for (i = 0; i < values->nvalues; i++) @@ -618,6 +619,25 @@ > virDomainCapsFeatureSEVFormat(virBuffer *buf, > return; > } > > +static void > +virDomainCapsFeatureSGXFormat(virBuffer *buf, > + const virSGXCapability *sgx) { > + if (!sgx) { > + return; // will delete in test patch No. The whole point of separating a feature into smaller patches is so that individual patches can be backported (plus it's easier for review). But this creates a dependency on the next commit, which removes this return. IOW, the next patch should be squashed in.

> + virBufferAddLit(buf, "<sgx supported='no'/>\n"); > + } else { > + return; // will delete in test patch > + virBufferAddLit(buf, "<sgx supported='yes'>\n"); > + virBufferAdjustIndent(buf, 2); > + virBufferAsprintf(buf, "<flc>%s</flc>\n", sgx->flc ? "yes" : "no"); > + virBufferAsprintf(buf, "<epc_size unit='KiB'>%d</epc_size>\n", sgx- >epc_size); > + virBufferAdjustIndent(buf, -2); > + virBufferAddLit(buf, "</sgx>\n"); > + } > + > + return; > +} > Michal