Hi Kai,
On Wed, 27 Sep 2023 06:14:20 -0500, Huang, Kai <kai.huang@xxxxxxxxx> wrote:
On Fri, 2023-09-22 at 20:06 -0700, Haitao Huang wrote:
From: Sean Christopherson <sean.j.christopherson@xxxxxxxxx>
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@xxxxxxxxx>
Co-developed-by: Kristen Carlson Accardi <kristen@xxxxxxxxxxxxxxx>
Signed-off-by: Kristen Carlson Accardi <kristen@xxxxxxxxxxxxxxx>
Co-developed-by: Haitao Huang <haitao.huang@xxxxxxxxxxxxxxx>
Signed-off-by: Haitao Huang <haitao.huang@xxxxxxxxxxxxxxx>
Cc: Sean Christopherson <seanjc@xxxxxxxxxx>
[...]
@@ -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.
As I mentioned in cover letter and change log in 11/18, this series does
not track virtual EPC.
We can add vepc field into the union in future if such tracking is needed.
Don't think "void *owner" is needed though.
Thanks
Haitao
