Messages in this thread | | | From | "Huang, Kai" <> | Subject | Re: [PATCH v5 09/18] x86/sgx: Store struct sgx_encl when allocating new VA pages | Date | Wed, 27 Sep 2023 11:14:20 +0000 |
| |
On Fri, 2023-09-22 at 20:06 -0700, Haitao Huang wrote: > From: Sean Christopherson <sean.j.christopherson@intel.com> > > In a later patch, when a cgroup has exceeded the max capacity for EPC > pages, it may need to identify and OOM kill a less active enclave to > make room for other enclaves within the same group. Such a victim > enclave would have no active pages other than the unreclaimable Version > Array (VA) and SECS pages. Therefore, the cgroup needs examine its > unreclaimable page list, and finding an enclave given a SECS page or a > VA page. This will require a backpointer from a page to an enclave, > which is not available for VA pages. > > Because struct sgx_epc_page instances of VA pages are not owned by an > sgx_encl_page instance, mark their owner as sgx_encl: pass the struct > sgx_encl of the enclave allocating the VA page to sgx_alloc_epc_page(), > which will store this value in the owner field of the struct > sgx_epc_page. In a later patch, VA pages will be placed in an > unreclaimable queue that can be examined by the cgroup to select the OOM > killed enclave. > > Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com> > Co-developed-by: Kristen Carlson Accardi <kristen@linux.intel.com> > Signed-off-by: Kristen Carlson Accardi <kristen@linux.intel.com> > Co-developed-by: Haitao Huang <haitao.huang@linux.intel.com> > Signed-off-by: Haitao Huang <haitao.huang@linux.intel.com> > Cc: Sean Christopherson <seanjc@google.com> >
[...]
> @@ -562,7 +562,7 @@ struct sgx_epc_page *sgx_alloc_epc_page(void *owner, bool reclaim) > for ( ; ; ) { > page = __sgx_alloc_epc_page(); > if (!IS_ERR(page)) { > - page->owner = owner; > + page->encl_page = owner;
Looks using 'encl_page' is arbitrary.
Also actually for virtual EPC page the owner is set to the 'sgx_vepc' instance.
> break; > } > > @@ -607,7 +607,7 @@ void sgx_free_epc_page(struct sgx_epc_page *page) > > spin_lock(&node->lock); > > - page->owner = NULL; > + page->encl_page = NULL;
Ditto.
> if (page->poison) > list_add(&page->list, &node->sgx_poison_page_list); > else > @@ -642,7 +642,7 @@ static bool __init sgx_setup_epc_section(u64 phys_addr, u64 size, > for (i = 0; i < nr_pages; i++) { > section->pages[i].section = index; > section->pages[i].flags = 0; > - section->pages[i].owner = NULL; > + section->pages[i].encl_page = NULL; > section->pages[i].poison = 0; > list_add_tail(§ion->pages[i].list, &sgx_dirty_page_list); > } > diff --git a/arch/x86/kernel/cpu/sgx/sgx.h b/arch/x86/kernel/cpu/sgx/sgx.h > index 764cec23f4e5..5110dd433b80 100644 > --- a/arch/x86/kernel/cpu/sgx/sgx.h > +++ b/arch/x86/kernel/cpu/sgx/sgx.h > @@ -68,7 +68,12 @@ struct sgx_epc_page { > unsigned int section; > u16 flags; > u16 poison; > - struct sgx_encl_page *owner; > + > + /* Possible owner types */ > + union { > + struct sgx_encl_page *encl_page; > + struct sgx_encl *encl; > + };
Sadly for virtual EPC page the owner is set to the 'sgx_vepc' instance it belongs to.
Given how sgx_{alloc|free}_epc_page() arbitrarily uses encl_page, perhaps we should do below?
union { struct sgx_encl_page *encl_page; struct sgx_encl *encl; struct sgx_vepc *vepc; void *owner; };
And in sgx_{alloc|free}_epc_page() we can use 'owner' instead.
| |