-----Original Message-----
From: Pavel Hrdina <phrdina(a)redhat.com>
Sent: Tuesday, July 20, 2021 5:29 PM
To: Daniel P. Berrangé <berrange(a)redhat.com>
Cc: Huang, Haibin <haibin.huang(a)intel.com>; libvir-list(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>; Peter Krempa <pkrempa(a)redhat.com>;
Michal Prívozník <mprivozn(a)redhat.com>
Subject: Re: [libvirt][PATCH v4 0/4] Support query and use SGX
On Tue, Jul 20, 2021 at 10:16:48AM +0100, Daniel P. Berrangé wrote:
> On Tue, Jul 20, 2021 at 10:47:27AM +0200, Pavel Hrdina wrote:
> > On Fri, Jul 16, 2021 at 12:58:19AM +0000, Huang, Haibin wrote:
> > >
> > > > -----Original Message-----
> > > > From: Pavel Hrdina <phrdina(a)redhat.com>
> > > > Sent: Wednesday, July 7, 2021 5:48 PM
> > > > To: Huang, Haibin <haibin.huang(a)intel.com>
> > > > Cc: libvir-list(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>
> > > > Subject: Re: [libvirt][PATCH v4 0/4] Support query and use SGX
> > > >
> > > > On Thu, Jul 01, 2021 at 08:10:25PM +0800, Haibin Huang wrote:
> > > > > This patch series provides support for enabling Intel's
> > > > > Software Guard
> > > > Extensions (SGX) feature in guest VM.
> > > > >
> > > > > Giving the SGX support in QEMU is still pending for reviewing,
> > > > > this patch series is not submmited for code review, but only
> > > > > describe the SGX enabling solution design that contains
> > > > > changes to
> > > > virConnectGetDomainCapabilities API response and domain
> > > > definition. All comments/suggestions would be highly appreciated.
> > > > >
> > > > > Intel Software Guard Extensions (Intel® SGX) is a set of
> > > > > instructions that increases the security of application code
> > > > > and data, giving them more protection from disclosure or
> > > > > modification. Developers can partition
> > > > sensitive information into enclaves, which are areas of
> > > > execution in memory with more security protection.
> > > > >
> > > > > The typical flow looks below at very high level:
> > > > >
> > > > > 1. Calls virConnectGetDomainCapabilities API to domain
> > > > > capabilities that
> > > > includes the following SGX information.
> > > > >
> > > > > <feature>
> > > > > ...
> > > > > <sgx supported='yes'>
> > > > > <epc_size unit=’KiB’>N</epc_size>
> > > > > </sgx>
> > > > > </feature>
> > > > >
> > > > > 2. User requests to start a guest calling virCreateXML() with
SGX
requirement.
> > > > > It should contain
> > > > >
> > > > > <launchSecurity type='sgx'>
> > > > > <epc_size unit='KiB'>N</epc_size>
</launchSecurity>
> > > >
> > > > I don't think that Intel SGX belongs into <launchSecurity>
in libvirt.
> > > > Similar feature to AMD SEV is Intel TDX which would be implement
> > > > using <launchSecurity> as it offers isolation between host and
VM.
> > > >
> > > > Looking at the patches this doesn't even use
> > > > confidential-guest-support machine option, it adds a new memory
> > > > backend and enables CPU features only if libvirt uses <cpu
mode='custom'> so it would not work with any other CPU mode.
> > > >
> > > > To me this sounds like we should split the feature into two
> > > > components where one would add support for the new memory
> > > > backend into correct XML part [1] and the other component would
> > > > be support for CPU features related to Intel SGX [2].
> > >
> > > [Haibin] ok, those specific CPU features we added have been deleted and
let user to specify it in [2].
> > > Do we need to add new element in memory backend for SGX EPC memory?
> >
> > Correct, reading QEMU and kernel patches to enable this feature in
> > libvirt user will need to configure SGX EPC memory backend manually.
> > However, we will not be able to reuse <memoryBacking> element in the
> > VM XML without a lot of modification to the current code. Mainly,
> > there can be mupltiple SGX EPC memory sections and each can have
different size.
> > Current code allows only single <memoryBacking> file and it is
> > closely tied with VM RAM.
> >
> > To express SGX EPC in VM XML we will need new element, for example
> > we can use <memory> device:
> >
> > <devices>
> > ...
> > <memory model='sgx-epc'>
> > <target>
> > <size unit='MiB'>64</size>
> > <node>0</node>
> > </target>
> > </memory>
> > ...
> > </devices>
> >
> > but this would require to modify the current <memory> code as the
> > 'sgx-epc' would be a special case where we would not use
'-device'
> > option because we need to add it to '-machine' parameter.
>
> Where are you seeing the -machine params ? In the patch 2 here
> it uses standalone parameters:
>
> -object memory-backend-epc,id=mem1,size=<epc_size>K,prealloc \
> -sgx-epc id=epc1,memdev=mem1
>
> which makes sense given you say that multiple SGX regions can be
> defined.
This RFC is a bit outdated, latest patches in QEMU dropped the new option '-sgx-
epc' and replaced it with compound -machine parameters [1].
This was explicitly requested by Paolo here [2].
> > Another option is to create completely new element, similar to
> > <launchSecurity> outside of <devices> element. I'm not sure
about
> > the naming of the new element, one thing that comes to my mind is
> > <memoryRegion> with type='sgx-epc'.
>
> I think adding a <memoryRegion> outside <devices> feels a little odd
> given that this parameter is defining new RAM blocks and we already
> have <memory> inside <devices>. I'd be more inclined towards the
> latter
Using <memory> was my first idea, I just wanted to offer some alternative as I
was not completely sure about using <memory> mainly because it will be part of
-machine option.
[Haibin] Can you guys confirm that putting <memory> in <device> is an
acceptable
solution? Even it will be translated to -machine instead of -device.
<devices>
...
<memory model='sgx-epc'>
<target>
<size unit='MiB'>64</size>
<node>0</node>
</target>
</memory>
...
</devices>