Message ID | 20210317235332.362001-1-jarkko.sakkinen@intel.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | [1/2] x86/sgx: Replace section->init_laundry_list with sgx_dirty_page_list | expand |
On Thu, Mar 18, 2021 at 01:53:30AM +0200, Jarkko Sakkinen wrote: > From: Jarkko Sakkinen <jarkko@kernel.org> > > During normal runtime, the "ksgxd" daemon behaves like a version of > kswapd just for SGX. But, before it starts acting like kswapd, its > first job is to initialize enclave memory. > > Currently, the SGX boot code places each enclave page on a > epc_section->init_laundry_list. Once it starts up, the ksgxd code walks > over that list and populates the actual SGX page allocator. > > However, the per-section structures are going away to make way for the SGX > NUMA allocator. There's also little need to have a per-section structure; > the enclave pages are all treated identically, and they can be placed on > the correct allocator list from metadata stored in the enclave page > (struct sgx_epc_page) itself. > > Modify sgx_sanitize_section() to take a single page list instead of taking > a section and deriving the list from there. > > Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org> > Acked-by: Dave Hansen <dave.hansen@linux.intel.com> > --- > > v5 > * Refine the commit message. > * Refine inline comments. > * Encapsulate a sanitization pass into __sgx_sanitize_pages(). > > v4: > * Open coded sgx_santize_section() to ksgxd(). > * Rewrote the commit message. > > arch/x86/kernel/cpu/sgx/main.c | 54 ++++++++++++++++------------------ > arch/x86/kernel/cpu/sgx/sgx.h | 7 ----- > 2 files changed, 25 insertions(+), 36 deletions(-) So both patches look ok to me but the sgx test case fails on -rc3 with and without those patches on my box: ./test_sgx 0x0000000000000000 0x0000000000002000 0x03 0x0000000000002000 0x0000000000001000 0x05 0x0000000000003000 0x0000000000003000 0x03 mmap() failed, errno=1. Box is: [ 0.138402] smpboot: CPU0: Intel(R) Core(TM) i5-9600K CPU @ 3.70GHz (family: 0x6, model: 0x9e, stepping: 0xc) [ 0.693947] sgx: EPC section 0x80200000-0x85ffffff And AFAIR that test used to pass there...
On 3/18/21 10:40 AM, Borislav Petkov wrote: > So both patches look ok to me but the sgx test case fails on -rc3 with and > without those patches on my box: > > ./test_sgx > 0x0000000000000000 0x0000000000002000 0x03 > 0x0000000000002000 0x0000000000001000 0x05 > 0x0000000000003000 0x0000000000003000 0x03 > mmap() failed, errno=1. I usually get that after I reboot. I have to do this: chmod 755 /dev/sgx_enclave mount -o remount,exec /dev BTW, that *IS* going to get tripped over all the time. The selftest needs a better error message. I'll send a patch.
On Thu, Mar 18, 2021 at 11:34:10AM -0700, Dave Hansen wrote: > I usually get that after I reboot. I have to do this: > > chmod 755 /dev/sgx_enclave > mount -o remount,exec /dev Yap, that did it: 0x0000000000000000 0x0000000000002000 0x03 0x0000000000002000 0x0000000000001000 0x05 0x0000000000003000 0x0000000000003000 0x03 SUCCESS > BTW, that *IS* going to get tripped over all the time. Yap, and I saw it last time and we talked about it and I forgot again. Conveniently. > The selftest needs a better error message. I'll send a patch. Please do. Thx.
On Thu, Mar 18, 2021 at 08:01:38PM +0100, Borislav Petkov wrote: > On Thu, Mar 18, 2021 at 11:34:10AM -0700, Dave Hansen wrote: > > I usually get that after I reboot. I have to do this: > > > > chmod 755 /dev/sgx_enclave > > mount -o remount,exec /dev > > Yap, that did it: > > 0x0000000000000000 0x0000000000002000 0x03 > 0x0000000000002000 0x0000000000001000 0x05 > 0x0000000000003000 0x0000000000003000 0x03 > SUCCESS > > > BTW, that *IS* going to get tripped over all the time. > > Yap, and I saw it last time and we talked about it and I forgot again. > Conveniently. > > > The selftest needs a better error message. I'll send a patch. > > Please do. Yeah, would make sense. Not longer than two or three weeks ago I wondered what is wrong with my system, up until I realized that I forgot to remount :-) Thanks for taking the patches. > Thx. /Jarkko
diff --git a/arch/x86/kernel/cpu/sgx/main.c b/arch/x86/kernel/cpu/sgx/main.c index 8df81a3ed945..f3a5cd2d27ef 100644 --- a/arch/x86/kernel/cpu/sgx/main.c +++ b/arch/x86/kernel/cpu/sgx/main.c @@ -26,39 +26,43 @@ static LIST_HEAD(sgx_active_page_list); static DEFINE_SPINLOCK(sgx_reclaimer_lock); +static LIST_HEAD(sgx_dirty_page_list); + /* - * Reset dirty EPC pages to uninitialized state. Laundry can be left with SECS - * pages whose child pages blocked EREMOVE. + * Reset post-kexec EPC pages to the uninitialized state. The pages are removed + * from the input list, and made available for the page allocator. SECS pages + * prepending their children in the input list are left intact. */ -static void sgx_sanitize_section(struct sgx_epc_section *section) +static void __sgx_sanitize_pages(struct list_head *dirty_page_list) { struct sgx_epc_page *page; LIST_HEAD(dirty); int ret; - /* init_laundry_list is thread-local, no need for a lock: */ - while (!list_empty(§ion->init_laundry_list)) { + /* dirty_page_list is thread-local, no need for a lock: */ + while (!list_empty(dirty_page_list)) { if (kthread_should_stop()) return; - /* needed for access to ->page_list: */ - spin_lock(§ion->lock); - - page = list_first_entry(§ion->init_laundry_list, - struct sgx_epc_page, list); + page = list_first_entry(dirty_page_list, struct sgx_epc_page, list); ret = __eremove(sgx_get_epc_virt_addr(page)); - if (!ret) - list_move(&page->list, §ion->page_list); - else + if (!ret) { + /* + * page is now sanitized. Make it available via the SGX + * page allocator: + */ + list_del(&page->list); + sgx_free_epc_page(page); + } else { + /* The page is not yet clean - move to the dirty list. */ list_move_tail(&page->list, &dirty); - - spin_unlock(§ion->lock); + } cond_resched(); } - list_splice(&dirty, §ion->init_laundry_list); + list_splice(&dirty, dirty_page_list); } static bool sgx_reclaimer_age(struct sgx_epc_page *epc_page) @@ -405,24 +409,17 @@ static bool sgx_should_reclaim(unsigned long watermark) static int ksgxd(void *p) { - int i; - set_freezable(); /* * Sanitize pages in order to recover from kexec(). The 2nd pass is * required for SECS pages, whose child pages blocked EREMOVE. */ - for (i = 0; i < sgx_nr_epc_sections; i++) - sgx_sanitize_section(&sgx_epc_sections[i]); - - for (i = 0; i < sgx_nr_epc_sections; i++) { - sgx_sanitize_section(&sgx_epc_sections[i]); + __sgx_sanitize_pages(&sgx_dirty_page_list); + __sgx_sanitize_pages(&sgx_dirty_page_list); - /* Should never happen. */ - if (!list_empty(&sgx_epc_sections[i].init_laundry_list)) - WARN(1, "EPC section %d has unsanitized pages.\n", i); - } + /* sanity check: */ + WARN_ON(!list_empty(&sgx_dirty_page_list)); while (!kthread_should_stop()) { if (try_to_freeze()) @@ -637,13 +634,12 @@ static bool __init sgx_setup_epc_section(u64 phys_addr, u64 size, section->phys_addr = phys_addr; spin_lock_init(§ion->lock); INIT_LIST_HEAD(§ion->page_list); - INIT_LIST_HEAD(§ion->init_laundry_list); for (i = 0; i < nr_pages; i++) { section->pages[i].section = index; section->pages[i].flags = 0; section->pages[i].owner = NULL; - list_add_tail(§ion->pages[i].list, §ion->init_laundry_list); + list_add_tail(§ion->pages[i].list, &sgx_dirty_page_list); } section->free_cnt = nr_pages; diff --git a/arch/x86/kernel/cpu/sgx/sgx.h b/arch/x86/kernel/cpu/sgx/sgx.h index 5fa42d143feb..bc8af0428640 100644 --- a/arch/x86/kernel/cpu/sgx/sgx.h +++ b/arch/x86/kernel/cpu/sgx/sgx.h @@ -45,13 +45,6 @@ struct sgx_epc_section { spinlock_t lock; struct list_head page_list; unsigned long free_cnt; - - /* - * Pages which need EREMOVE run on them before they can be - * used. Only safe to be accessed in ksgxd and init code. - * Not protected by locks. - */ - struct list_head init_laundry_list; }; extern struct sgx_epc_section sgx_epc_sections[SGX_MAX_EPC_SECTIONS];