Re: [PATCH v2] x86/sgx: Hack in idea for allocating from local EPC node when possible

[Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>]

On Thu, May 14, 2020 at 09:30:24AM +0300, Jarkko Sakkinen wrote:
> On Wed, May 13, 2020 at 10:10:36PM -0700, Sean Christopherson wrote:
> > Allocate EPC from the local node when possible.  There is no new NUMA
> > enumeration for EPC.  Because EPC is carved out of RAM on bare metal,
> > the sections are naturally covered by the existing ACPI SRAT entries,
> > i.e. can be found by querying the kernel's NUMA info.
> > 
> > Keep the per-section tracking to simplify iterating over all sections
> > and reverse lookups given an EPC page.
> > 
> > Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com>
> > ---
> > 
> > I like this version a lot more.  Far less clever and doesn't assume
> > anything about number of EPC sections vs. NUMA nodes.
> > 
> > As before, compile tested only.
> > 
> > For folks (especially non-Intel people) who may be confused, this is a
> > less-than-an-RFC patch to frame in an idea for adding basic NUMA awareness
> > for EPC sections without (yet) supporting full mempolicy stuff.
> > 
> >  arch/x86/kernel/cpu/sgx/main.c    | 95 +++++++++++++++++++++++++------
> >  arch/x86/kernel/cpu/sgx/reclaim.c |  6 +-
> >  arch/x86/kernel/cpu/sgx/sgx.h     |  6 +-
> >  3 files changed, 84 insertions(+), 23 deletions(-)
> > 
> > diff --git a/arch/x86/kernel/cpu/sgx/main.c b/arch/x86/kernel/cpu/sgx/main.c
> > index 5ce77e5546766..3128b4fa5ff4f 100644
> > --- a/arch/x86/kernel/cpu/sgx/main.c
> > +++ b/arch/x86/kernel/cpu/sgx/main.c
> > @@ -11,7 +11,15 @@
> >  #include "driver.h"
> >  #include "encls.h"
> >  
> > -struct sgx_epc_section sgx_epc_sections[SGX_MAX_EPC_SECTIONS];
> > +struct sgx_epc_node {
> > +	struct sgx_epc_section sections[SGX_MAX_EPC_SECTIONS];
> > +	int nr_sections;
> > +};
> > +
> > +static struct sgx_epc_node sgx_epc_nodes[MAX_NUMNODES];
> > +static int sgx_nr_epc_nodes;
> > +
> > +struct sgx_epc_section *sgx_epc_sections[SGX_MAX_EPC_SECTIONS];
> >  int sgx_nr_epc_sections;
> >  
> >  static struct sgx_epc_page *__sgx_try_alloc_page(struct sgx_epc_section *section)
> > @@ -28,23 +36,15 @@ static struct sgx_epc_page *__sgx_try_alloc_page(struct sgx_epc_section *section
> >  	return page;
> >  }
> >  
> > -/**
> > - * sgx_try_alloc_page() - Allocate an EPC page
> > - *
> > - * Try to grab a page from the free EPC page list.
> > - *
> > - * Return:
> > - *   a pointer to a &struct sgx_epc_page instance,
> > - *   -errno on error
> > - */
> > -struct sgx_epc_page *sgx_try_alloc_page(void)
> > +static struct sgx_epc_page *sgx_try_alloc_page_node(int nid)
> >  {
> > +	struct sgx_epc_node *node = &sgx_epc_nodes[nid];
> >  	struct sgx_epc_section *section;
> >  	struct sgx_epc_page *page;
> >  	int i;
> >  
> > -	for (i = 0; i < sgx_nr_epc_sections; i++) {
> > -		section = &sgx_epc_sections[i];
> > +	for (i = 0; i < node->nr_sections; i++) {
> > +		section = &node->sections[i];
> >  		spin_lock(&section->lock);
> >  		page = __sgx_try_alloc_page(section);
> >  		spin_unlock(&section->lock);
> > @@ -53,6 +53,41 @@ struct sgx_epc_page *sgx_try_alloc_page(void)
> >  			return page;
> >  	}
> >  
> > +	return NULL;
> > +}
> > +
> > +/**
> > + * sgx_try_alloc_page() - Allocate an EPC page
> > + *
> > + * Try to grab a page from the free EPC page list.
> > + *
> > + * Return:
> > + *   a pointer to a &struct sgx_epc_page instance,
> > + *   -errno on error
> > + */
> > +struct sgx_epc_page *sgx_try_alloc_page(void)
> > +{
> > +	struct sgx_epc_page *page;
> > +	int nid = numa_node_id();
> 
> This means that CONFIG_NUMA is not really needed.

I'll refine the patch with this premise, should turn out to be somewhat
simple.

/Jarkko