Re: [PATCH v13 09/13] x86/sgx: Enclave Page Cache (EPC) memory manager

[Sean Christopherson]

On Tue, Aug 28, 2018 at 07:07:33AM -0700, Dave Hansen wrote:
> On 08/28/2018 01:35 AM, Jarkko Sakkinen wrote:
> > On Mon, Aug 27, 2018 at 02:15:34PM -0700, Dave Hansen wrote:
> >> On 08/27/2018 11:53 AM, Jarkko Sakkinen wrote:
> >>> +struct sgx_epc_page_ops {
> >>> +	bool (*get)(struct sgx_epc_page *epc_page);
> >>> +	void (*put)(struct sgx_epc_page *epc_page);
> >>> +	bool (*reclaim)(struct sgx_epc_page *epc_page);
> >>> +	void (*block)(struct sgx_epc_page *epc_page);
> >>> +	void (*write)(struct sgx_epc_page *epc_page);
> >>> +};
> >> Why do we need a fancy, slow (retpoline'd) set of function pointers when
> >> we only have one user of these (the SGX driver)?
> > KVM has its own implementation for these operations.
> 
> That belongs in the changelog.
> 
> Also, where is the implementation?  How can we assess this code that was
> built to create an abstraction without both of the users?

I can provide an early preview of the KVM reclaim code, but honestly
I think that would do more harm than good.  The VMX architecture for
EPC reclaim is complex, even for SGX standards.  Opening that can of
worms would likely derail this discussion.  That being said, this
abstraction isn't exactly what KVM will need, but it's pretty close
and gives us something to build on.

Regardless, this layer of indirection is justifiable even with a
single implementation.  Reclaiming an EPC page is not a simple matter
of copying the data somewhere else and marking the page not present.
Actual eviction requires a reference to the Secure Enclave Control
Structure (SECS) of the enclave that owns the page.  Software needs
to ensure it doesn't violate the hardware-enforced access rules, e.g.
most reclaim activites need to be done under a per-enclave lock.
Not all pages have the same reclaim rules, e.g. an SECS can only
be reclaimed after all its child pages have reclaimed, a VA page
doesn't need to be blocked, etc...  And the list goes on...

All of the tracking metadata and logic about what can be evicted when
resides in the driver component[1], e.g. the core SGX code doesn't
even have a software representation of an enclave.  Alternatively the
driver could provide a single "do_reclaim" function if we wanted to
avoid a fancy abstraction layer, and in fact that's how I initially
implemented the abstraction, but that approach has it's own warts and
in a lot of ways makes the end result more complex.

Ultimately, moving pages in/out of the EPC is so abysmally slow that
the raw performance of software is almost completely irrelevant.  The
algorithms certainly matter, but optimizing away things like function
pointers definitely takes a back seat to just about everything else.

[1] Early versions of SGX support tried to put all EPC management in
    the driver, but that approach caused major problems for KVM (even
    without reclaim support) and received a less than enthusiastic
    response from the community.