Re: [libvirt][PATCH v4 0/4] Support query and use SGX

> -----Original Message----- > From: Pavel Hrdina <phrdina(a)redhat.com&gt; > Sent: Tuesday, July 20, 2021 5:29 PM > To: Daniel P. Berrangé <berrange(a)redhat.com&gt; > 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&gt; > 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&gt; > > > > > Sent: Wednesday, July 7, 2021 5:48 PM > > > > > To: Huang, Haibin <haibin.huang(a)intel.com&gt; > > > > > 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&gt; > > > > > 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>