On Wed, Jul 07, 2021 at 11:47:37AM +0200, Pavel Hrdina wrote: > 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. This just looks like a bug - there's no reason I see why it shouldn't work with all CPU modes. In fact the user could just specify the <feature> elements under <cpu> using existing syntax. We just need the cpu map to know about them > 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]. Yeah, sounds more sensible Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|