| Message ID | 20230712230202.47929-3-haitao.huang@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add Cgroup support for SGX EPC memory | expand |
On Wed Jul 12, 2023 at 11:01 PM UTC, Haitao Huang wrote: > From: Sean Christopherson <sean.j.christopherson@intel.com> > > Two types of owners, 'sgx_encl' for VA pages and 'sgx_encl_page' for other, > can be stored in the union field in sgx_epc_page struct introduced in the > previous patch. This would be easier to follow: "Two types of owners of struct_epc_page, 'sgx_encl' for VA pages and 'sgx_encl_page' can be stored in the previously introduced union field." > When cgroup OOM support is added in a later patch, the owning enclave of a > page will need to be identified. Retrieving the sgx_encl struct from a > sgx_epc_page will be different if the page is a VA page vs. other enclave > pages. > > Add 2 flags which will identify the type of the owner and apply them > accordingly to newly allocated pages. This would be easier to follow: "OOM support for cgroups requires that the owner needs to be identified when selecting pages from the unreclaimable list. Address this by adding flags for identifying the owner type." It is better to carry the story a little bit forward than say that a subsequent patch will require this :-) I.e. enough to get at least a rough idea what is going on. R, Jarkko
On Mon Jul 17, 2023 at 12:41 PM UTC, Jarkko Sakkinen wrote: > On Wed Jul 12, 2023 at 11:01 PM UTC, Haitao Huang wrote: > > From: Sean Christopherson <sean.j.christopherson@intel.com> > > > > Two types of owners, 'sgx_encl' for VA pages and 'sgx_encl_page' for other, > > can be stored in the union field in sgx_epc_page struct introduced in the > > previous patch. > > This would be easier to follow: > > "Two types of owners of struct_epc_page, 'sgx_encl' for VA pages and > 'sgx_encl_page' can be stored in the previously introduced union field." > > > When cgroup OOM support is added in a later patch, the owning enclave of a > > page will need to be identified. Retrieving the sgx_encl struct from a > > sgx_epc_page will be different if the page is a VA page vs. other enclave > > pages. > > > > Add 2 flags which will identify the type of the owner and apply them > > accordingly to newly allocated pages. > > This would be easier to follow: > > "OOM support for cgroups requires that the owner needs to be identified > when selecting pages from the unreclaimable list. Address this by adding > flags for identifying the owner type." > > It is better to carry the story a little bit forward than say that a > subsequent patch will require this :-) I.e. enough to get at least a > rough idea what is going on. Oops, sent by mistake. I was going to say that the flag would be better named simply as SGX_EPC_OWNER_PAGE instead of SGX_EPC_OWNER_ENCL_PAGE. BR, Jarkko
diff --git a/arch/x86/kernel/cpu/sgx/encl.c b/arch/x86/kernel/cpu/sgx/encl.c index 98e1086eab07..3bc2f95b1da2 100644 --- a/arch/x86/kernel/cpu/sgx/encl.c +++ b/arch/x86/kernel/cpu/sgx/encl.c @@ -252,6 +252,7 @@ static struct sgx_encl_page *__sgx_encl_load_page(struct sgx_encl *encl, epc_page = sgx_encl_eldu(&encl->secs, NULL); if (IS_ERR(epc_page)) return ERR_CAST(epc_page); + epc_page->flags |= SGX_EPC_OWNER_ENCL_PAGE; } epc_page = sgx_encl_eldu(entry, encl->secs.epc_page); @@ -260,6 +261,7 @@ static struct sgx_encl_page *__sgx_encl_load_page(struct sgx_encl *encl, encl->secs_child_cnt++; sgx_mark_page_reclaimable(entry->epc_page); + entry->epc_page->flags |= SGX_EPC_OWNER_ENCL_PAGE; return entry; } @@ -379,6 +381,7 @@ static vm_fault_t sgx_encl_eaug_page(struct vm_area_struct *vma, encl->secs_child_cnt++; sgx_mark_page_reclaimable(encl_page->epc_page); + encl_page->epc_page->flags |= SGX_EPC_OWNER_ENCL_PAGE; phys_addr = sgx_get_epc_phys_addr(epc_page); /* @@ -1235,6 +1238,7 @@ struct sgx_epc_page *sgx_alloc_va_page(struct sgx_encl *encl, bool reclaim) sgx_encl_free_epc_page(epc_page); return ERR_PTR(-EFAULT); } + epc_page->flags |= SGX_EPC_OWNER_ENCL; return epc_page; } diff --git a/arch/x86/kernel/cpu/sgx/ioctl.c b/arch/x86/kernel/cpu/sgx/ioctl.c index fa8c3f32ccf6..fe3e89cf013f 100644 --- a/arch/x86/kernel/cpu/sgx/ioctl.c +++ b/arch/x86/kernel/cpu/sgx/ioctl.c @@ -113,6 +113,8 @@ static int sgx_encl_create(struct sgx_encl *encl, struct sgx_secs *secs) encl->attributes = secs->attributes; encl->attributes_mask = SGX_ATTR_UNPRIV_MASK; + encl->secs.epc_page->flags |= SGX_EPC_OWNER_ENCL_PAGE; + /* Set only after completion, as encl->lock has not been taken. */ set_bit(SGX_ENCL_CREATED, &encl->flags); @@ -323,6 +325,7 @@ static int sgx_encl_add_page(struct sgx_encl *encl, unsigned long src, } sgx_mark_page_reclaimable(encl_page->epc_page); + encl_page->epc_page->flags |= SGX_EPC_OWNER_ENCL_PAGE; mutex_unlock(&encl->lock); mmap_read_unlock(current->mm); return ret; @@ -977,6 +980,7 @@ static long sgx_enclave_modify_types(struct sgx_encl *encl, mutex_lock(&encl->lock); sgx_mark_page_reclaimable(entry->epc_page); + entry->epc_page->flags |= SGX_EPC_OWNER_ENCL_PAGE; } /* Change EPC type */ diff --git a/arch/x86/kernel/cpu/sgx/sgx.h b/arch/x86/kernel/cpu/sgx/sgx.h index dc1cbcfcf2d4..f6e3c5810eef 100644 --- a/arch/x86/kernel/cpu/sgx/sgx.h +++ b/arch/x86/kernel/cpu/sgx/sgx.h @@ -29,6 +29,12 @@ /* Pages on free list */ #define SGX_EPC_PAGE_IS_FREE BIT(1) +/* flag for pages owned by a sgx_encl_page */ +#define SGX_EPC_OWNER_ENCL_PAGE BIT(3) + +/* flag for pages owned by a sgx_encl struct */ +#define SGX_EPC_OWNER_ENCL BIT(4) + struct sgx_epc_page { unsigned int section; u16 flags;