Re: [RFC PATCH 03/23] x86/sgx: Introduce virtual EPC for use by KVM guests

[Jarkko Sakkinen <jarkko@kernel.org>]

On Tue, Jan 12, 2021 at 01:56:54PM +1300, Kai Huang wrote:
> On Tue, 12 Jan 2021 01:38:23 +0200 Jarkko Sakkinen wrote:
> > On Wed, Jan 06, 2021 at 02:55:20PM +1300, Kai Huang wrote:
> > > From: Sean Christopherson <sean.j.christopherson@intel.com>
> > > 
> > > Add a misc device /dev/sgx_virt_epc to allow userspace to allocate "raw"
> > > EPC without an associated enclave.  The intended and only known use case
> > > for raw EPC allocation is to expose EPC to a KVM guest, hence the
> > > virt_epc moniker, virt.{c,h} files and X86_SGX_VIRTUALIZATION Kconfig.
> > > 
> > > Modify sgx_init() to always try to initialize virtual EPC driver, even
> > > when SGX driver is disabled due to SGX Launch Control is in locked mode,
> > > or not present at all, since SGX virtualization allows to expose SGX to
> > > guests that support non-LC configurations.
> > > 
> > > Implement the "raw" EPC allocation in the x86 core-SGX subsystem via
> > > /dev/sgx_virt_epc rather than in KVM. Doing so has two major advantages:
> > > 
> > >   - Does not require changes to KVM's uAPI, e.g. EPC gets handled as
> > >     just another memory backend for guests.
> > > 
> > >   - EPC management is wholly contained in the SGX subsystem, e.g. SGX
> > >     does not have to export any symbols, changes to reclaim flows don't
> > >     need to be routed through KVM, SGX's dirty laundry doesn't have to
> > >     get aired out for the world to see, and so on and so forth.
> > > 
> > > The virtual EPC allocated to guests is currently not reclaimable, due to
> > > oversubscription of EPC for KVM guests is not currently supported. Due
> > > to the complications of handling reclaim conflicts between guest and
> > > host, KVM EPC oversubscription is significantly more complex than basic
> > > support for SGX virtualization.
> > > 
> > > Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
> > > Co-developed-by: Kai Huang <kai.huang@intel.com>
> > > Signed-off-by: Kai Huang <kai.huang@intel.com>
> > 
> > The commit message does not describe the code changes. It should
> > have an understandable explanation of fops. There is nothing about
> > the implementation right now.
> 
> Thanks for feedback. Does "understabdable explanation of fops" mean I
> should add one sentence to say, for instance: "userspace hypervisor should open
> the /dev/sgx_virt_epc, use mmap() to get a valid address range, and then use
> that address range to create KVM memory region"?
> 
> Or should I include an example of how to use /dev/sgx_virt_epc in userspace, for
> instance, below?
> 
> 	fd = open("/dev/sgx_virt_epc", O_RDWR);
> 	void *addr = mmap(NULL, size, ..., fd);
> 	/* userspace hypervisor uses addr, size to create KVM memory slot */
> 	...

I would suggest just to describe them in few sentences. Just write
how you understand them in one paragraph.

/Jarkko