| Message ID | 20170404072938.4800-3-kai.huang@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Apr 04, 2017 at 07:29:33PM +1200, Kai Huang wrote: > Currently sgx_page_cache_init is called for each EPC bank in case of multiple > EPC banks, which is wrong because sgx_page_cache_init creates ksgxswapd kernel > thread which should only be created once in global. This patch refines page > cache initlization code by introducing sgx_epc_bank_init which initializes one > EPC bank, and sgx_page_cache_init is changed to take all EPC banks as parameter > and initialize them one by one by calling sgx_epc_bank_init. And ksgxswapd will > be created once in sgx_page_cache_init. ioremap for EPC bank is also moved to > sgx_epc_bank_init. This is more reasonable, as EPC page's virtual address > handling is also part of page cache management code. > > Signed-off-by: Kai Huang <kai.huang@linux.intel.com> I haven't yet even reviewed the earlier version. Is this mandatory for this patch set and continuing KVM development? /Jarkko > --- > drivers/platform/x86/intel_sgx.h | 4 +- > drivers/platform/x86/intel_sgx_main.c | 40 +++------------- > drivers/platform/x86/intel_sgx_page_cache.c | 71 ++++++++++++++++++++++++++--- > 3 files changed, 73 insertions(+), 42 deletions(-) > > diff --git a/drivers/platform/x86/intel_sgx.h b/drivers/platform/x86/intel_sgx.h > index adb5b17..a98915a 100644 > --- a/drivers/platform/x86/intel_sgx.h > +++ b/drivers/platform/x86/intel_sgx.h > @@ -252,8 +252,8 @@ enum sgx_free_flags { > }; > > int ksgxswapd(void *p); > -int sgx_page_cache_init(resource_size_t start, unsigned long size); > -void sgx_page_cache_teardown(void); > +int sgx_page_cache_init(struct sgx_epc_bank *banks, int nr_banks); > +void sgx_page_cache_teardown(struct sgx_epc_bank *banks, int nr_banks); > struct sgx_epc_page *sgx_alloc_page(struct sgx_tgid_ctx *tgid_epc_cnt, > unsigned int flags); > int sgx_free_page(struct sgx_epc_page *entry, struct sgx_encl *encl, > diff --git a/drivers/platform/x86/intel_sgx_main.c b/drivers/platform/x86/intel_sgx_main.c > index 6c2d240..e242969 100644 > --- a/drivers/platform/x86/intel_sgx_main.c > +++ b/drivers/platform/x86/intel_sgx_main.c > @@ -264,7 +264,6 @@ static int sgx_dev_init(struct device *dev) > { > unsigned int wq_flags; > int ret; > - int i; > > pr_info("intel_sgx: " DRV_DESCRIPTION " v" DRV_VERSION "\n"); > > @@ -277,25 +276,9 @@ static int sgx_dev_init(struct device *dev) > > pr_info("intel_sgx: Number of EPCs %d\n", sgx_nr_epc_banks); > > - for (i = 0; i < sgx_nr_epc_banks; i++) { > - pr_info("intel_sgx: EPC memory range 0x%lx-0x%lx\n", > - sgx_epc_banks[i].start, sgx_epc_banks[i].end); > -#ifdef CONFIG_X86_64 > - sgx_epc_banks[i].mem = ioremap_cache(sgx_epc_banks[i].start, > - sgx_epc_banks[i].end - sgx_epc_banks[i].start); > - if (!sgx_epc_banks[i].mem) { > - sgx_nr_epc_banks = i; > - ret = -ENOMEM; > - goto out_iounmap; > - } > -#endif > - ret = sgx_page_cache_init(sgx_epc_banks[i].start, > - sgx_epc_banks[i].end - sgx_epc_banks[i].start); > - if (ret) { > - sgx_nr_epc_banks = i+1; > - goto out_iounmap; > - } > - } > + ret = sgx_page_cache_init(sgx_epc_banks, sgx_nr_epc_banks); > + if (ret) > + return ret; > > wq_flags = WQ_UNBOUND | WQ_FREEZABLE; > #ifdef WQ_NON_REENETRANT > @@ -305,7 +288,7 @@ static int sgx_dev_init(struct device *dev) > if (!sgx_add_page_wq) { > pr_err("intel_sgx: alloc_workqueue() failed\n"); > ret = -ENOMEM; > - goto out_iounmap; > + goto out_page_cache_init; > } > > sgx_dev.parent = dev; > @@ -318,11 +301,8 @@ static int sgx_dev_init(struct device *dev) > return 0; > out_workqueue: > destroy_workqueue(sgx_add_page_wq); > -out_iounmap: > -#ifdef CONFIG_X86_64 > - for (i = 0; i < sgx_nr_epc_banks; i++) > - iounmap(sgx_epc_banks[i].mem); > -#endif > +out_page_cache_init: > + sgx_page_cache_teardown(sgx_epc_banks, sgx_nr_epc_banks); > return ret; > } > > @@ -378,15 +358,9 @@ static int sgx_drv_probe(struct platform_device *pdev) > > static int sgx_drv_remove(struct platform_device *pdev) > { > - int i; > - > misc_deregister(&sgx_dev); > destroy_workqueue(sgx_add_page_wq); > -#ifdef CONFIG_X86_64 > - for (i = 0; i < sgx_nr_epc_banks; i++) > - iounmap(sgx_epc_banks[i].mem); > -#endif > - sgx_page_cache_teardown(); > + sgx_page_cache_teardown(sgx_epc_banks, sgx_nr_epc_banks); > > return 0; > } > diff --git a/drivers/platform/x86/intel_sgx_page_cache.c b/drivers/platform/x86/intel_sgx_page_cache.c > index 852066c..2a277ba 100644 > --- a/drivers/platform/x86/intel_sgx_page_cache.c > +++ b/drivers/platform/x86/intel_sgx_page_cache.c > @@ -420,17 +420,31 @@ int ksgxswapd(void *p) > return 0; > } > > -int sgx_page_cache_init(resource_size_t start, unsigned long size) > +static int sgx_epc_bank_init(struct sgx_epc_bank *bank) > { > - unsigned long i; > + unsigned long i, size; > struct sgx_epc_page *new_epc_page, *entry; > struct list_head *parser, *temp; > > + pr_info("intel_sgx: EPC memory range 0x%lx-0x%lx\n", > + bank->start, bank->end); > + > + if (!bank->start || !bank->end) > + return -ENODEV; > + > + size = bank->end - bank->start; > + > +#ifdef CONFIG_X86_64 > + bank->mem = ioremap_cache(bank->start, size); > + if (!bank->mem) > + return -ENOMEM; > +#endif > + > for (i = 0; i < size; i += PAGE_SIZE) { > new_epc_page = kzalloc(sizeof(*new_epc_page), GFP_KERNEL); > if (!new_epc_page) > goto err_freelist; > - new_epc_page->pa = start + i; > + new_epc_page->pa = bank->start + i; > > spin_lock(&sgx_free_list_lock); > list_add_tail(&new_epc_page->free_list, &sgx_free_list); > @@ -439,10 +453,8 @@ int sgx_page_cache_init(resource_size_t start, unsigned long size) > spin_unlock(&sgx_free_list_lock); > } > > - sgx_nr_high_pages = 2 * sgx_nr_low_pages; > - ksgxswapd_tsk = kthread_run(ksgxswapd, NULL, "ksgxswapd"); > - > return 0; > + > err_freelist: > list_for_each_safe(parser, temp, &sgx_free_list) { > spin_lock(&sgx_free_list_lock); > @@ -451,13 +463,55 @@ int sgx_page_cache_init(resource_size_t start, unsigned long size) > spin_unlock(&sgx_free_list_lock); > kfree(entry); > } > +#ifdef CONFIG_X86_64 > + iounmap(bank->mem); > +#endif > return -ENOMEM; > } > > -void sgx_page_cache_teardown(void) > +static void sgx_epc_bank_teardown(struct sgx_epc_bank *bank) > +{ > + if (!bank->start || !bank->end) > + return; > + > + /* > + * Don't free 'struct sgx_epc_page' for EPC page in this bank. All > + * 'struct sgx_epc_page' for all EPC pages will be destroyed together. > + */ > +#ifdef CONFIG_X86_64 > + if (bank->mem) > + iounmap(bank->mem); > +#endif > +} > + > +int sgx_page_cache_init(struct sgx_epc_bank *banks, int nr_banks) > +{ > + int i, ret = -ENODEV; > + > + for (i = 0; i < nr_banks; i++) { > + ret = sgx_epc_bank_init(banks + i); > + if (ret) > + break; > + } > + > + if (!i) > + return ret; > + > + /* > + * We can continue to use EPC banks that have been initialized > + * successfully, even we fail on initializing one particular EPC bank. > + */ > + sgx_nr_high_pages = 2 * sgx_nr_low_pages; > + ksgxswapd_tsk = kthread_run(ksgxswapd, NULL, "ksgxswapd"); > + > + return 0; > +} > + > +void sgx_page_cache_teardown(struct sgx_epc_bank *banks, int nr_banks) > { > struct sgx_epc_page *entry; > struct list_head *parser, *temp; > + int i; > > if (ksgxswapd_tsk) > kthread_stop(ksgxswapd_tsk); > @@ -469,6 +523,9 @@ void sgx_page_cache_teardown(void) > kfree(entry); > } > spin_unlock(&sgx_free_list_lock); > + > + for (i = 0; i < nr_banks; i++) > + sgx_epc_bank_teardown(banks + i); > } > > static struct sgx_epc_page *sgx_alloc_page_fast(void) > -- > 2.9.3 >
On 4/5/2017 2:50 AM, Jarkko Sakkinen wrote: > On Tue, Apr 04, 2017 at 07:29:33PM +1200, Kai Huang wrote: >> Currently sgx_page_cache_init is called for each EPC bank in case of multiple >> EPC banks, which is wrong because sgx_page_cache_init creates ksgxswapd kernel >> thread which should only be created once in global. This patch refines page >> cache initlization code by introducing sgx_epc_bank_init which initializes one >> EPC bank, and sgx_page_cache_init is changed to take all EPC banks as parameter >> and initialize them one by one by calling sgx_epc_bank_init. And ksgxswapd will >> be created once in sgx_page_cache_init. ioremap for EPC bank is also moved to >> sgx_epc_bank_init. This is more reasonable, as EPC page's virtual address >> handling is also part of page cache management code. >> >> Signed-off-by: Kai Huang <kai.huang@linux.intel.com> > > I haven't yet even reviewed the earlier version. Is this mandatory for > this patch set and continuing KVM development? The earlier one and this one are 99% identical, and you can review either of them. Patch 1, 3,4,5 in this series are mandatory for KVM, but patch 3,4,5 are based on this patch. This patch fixes the multiple EPC banks handling bug in existing code. If you don't mind keeping this bug then I am fine to drop this one and rewrite patch 3,4,5 based on existing code. Or like you suggested for cover letter, I probably should send this patch separately, which is exactly what I did before sending out this series (the earlier one) btw. I included it in this series as patch 3,4,5 depend on it. > > /Jarkko > >> --- >> drivers/platform/x86/intel_sgx.h | 4 +- >> drivers/platform/x86/intel_sgx_main.c | 40 +++------------- >> drivers/platform/x86/intel_sgx_page_cache.c | 71 ++++++++++++++++++++++++++--- >> 3 files changed, 73 insertions(+), 42 deletions(-) >> >> diff --git a/drivers/platform/x86/intel_sgx.h b/drivers/platform/x86/intel_sgx.h >> index adb5b17..a98915a 100644 >> --- a/drivers/platform/x86/intel_sgx.h >> +++ b/drivers/platform/x86/intel_sgx.h >> @@ -252,8 +252,8 @@ enum sgx_free_flags { >> }; >> >> int ksgxswapd(void *p); >> -int sgx_page_cache_init(resource_size_t start, unsigned long size); >> -void sgx_page_cache_teardown(void); >> +int sgx_page_cache_init(struct sgx_epc_bank *banks, int nr_banks); >> +void sgx_page_cache_teardown(struct sgx_epc_bank *banks, int nr_banks); >> struct sgx_epc_page *sgx_alloc_page(struct sgx_tgid_ctx *tgid_epc_cnt, >> unsigned int flags); >> int sgx_free_page(struct sgx_epc_page *entry, struct sgx_encl *encl, >> diff --git a/drivers/platform/x86/intel_sgx_main.c b/drivers/platform/x86/intel_sgx_main.c >> index 6c2d240..e242969 100644 >> --- a/drivers/platform/x86/intel_sgx_main.c >> +++ b/drivers/platform/x86/intel_sgx_main.c >> @@ -264,7 +264,6 @@ static int sgx_dev_init(struct device *dev) >> { >> unsigned int wq_flags; >> int ret; >> - int i; >> >> pr_info("intel_sgx: " DRV_DESCRIPTION " v" DRV_VERSION "\n"); >> >> @@ -277,25 +276,9 @@ static int sgx_dev_init(struct device *dev) >> >> pr_info("intel_sgx: Number of EPCs %d\n", sgx_nr_epc_banks); >> >> - for (i = 0; i < sgx_nr_epc_banks; i++) { >> - pr_info("intel_sgx: EPC memory range 0x%lx-0x%lx\n", >> - sgx_epc_banks[i].start, sgx_epc_banks[i].end); >> -#ifdef CONFIG_X86_64 >> - sgx_epc_banks[i].mem = ioremap_cache(sgx_epc_banks[i].start, >> - sgx_epc_banks[i].end - sgx_epc_banks[i].start); >> - if (!sgx_epc_banks[i].mem) { >> - sgx_nr_epc_banks = i; >> - ret = -ENOMEM; >> - goto out_iounmap; >> - } >> -#endif >> - ret = sgx_page_cache_init(sgx_epc_banks[i].start, >> - sgx_epc_banks[i].end - sgx_epc_banks[i].start); >> - if (ret) { >> - sgx_nr_epc_banks = i+1; >> - goto out_iounmap; >> - } >> - } >> + ret = sgx_page_cache_init(sgx_epc_banks, sgx_nr_epc_banks); >> + if (ret) >> + return ret; >> >> wq_flags = WQ_UNBOUND | WQ_FREEZABLE; >> #ifdef WQ_NON_REENETRANT >> @@ -305,7 +288,7 @@ static int sgx_dev_init(struct device *dev) >> if (!sgx_add_page_wq) { >> pr_err("intel_sgx: alloc_workqueue() failed\n"); >> ret = -ENOMEM; >> - goto out_iounmap; >> + goto out_page_cache_init; >> } >> >> sgx_dev.parent = dev; >> @@ -318,11 +301,8 @@ static int sgx_dev_init(struct device *dev) >> return 0; >> out_workqueue: >> destroy_workqueue(sgx_add_page_wq); >> -out_iounmap: >> -#ifdef CONFIG_X86_64 >> - for (i = 0; i < sgx_nr_epc_banks; i++) >> - iounmap(sgx_epc_banks[i].mem); >> -#endif >> +out_page_cache_init: >> + sgx_page_cache_teardown(sgx_epc_banks, sgx_nr_epc_banks); >> return ret; >> } >> >> @@ -378,15 +358,9 @@ static int sgx_drv_probe(struct platform_device *pdev) >> >> static int sgx_drv_remove(struct platform_device *pdev) >> { >> - int i; >> - >> misc_deregister(&sgx_dev); >> destroy_workqueue(sgx_add_page_wq); >> -#ifdef CONFIG_X86_64 >> - for (i = 0; i < sgx_nr_epc_banks; i++) >> - iounmap(sgx_epc_banks[i].mem); >> -#endif >> - sgx_page_cache_teardown(); >> + sgx_page_cache_teardown(sgx_epc_banks, sgx_nr_epc_banks); >> >> return 0; >> } >> diff --git a/drivers/platform/x86/intel_sgx_page_cache.c b/drivers/platform/x86/intel_sgx_page_cache.c >> index 852066c..2a277ba 100644 >> --- a/drivers/platform/x86/intel_sgx_page_cache.c >> +++ b/drivers/platform/x86/intel_sgx_page_cache.c >> @@ -420,17 +420,31 @@ int ksgxswapd(void *p) >> return 0; >> } >> >> -int sgx_page_cache_init(resource_size_t start, unsigned long size) >> +static int sgx_epc_bank_init(struct sgx_epc_bank *bank) >> { >> - unsigned long i; >> + unsigned long i, size; >> struct sgx_epc_page *new_epc_page, *entry; >> struct list_head *parser, *temp; >> >> + pr_info("intel_sgx: EPC memory range 0x%lx-0x%lx\n", >> + bank->start, bank->end); >> + >> + if (!bank->start || !bank->end) >> + return -ENODEV; >> + >> + size = bank->end - bank->start; >> + >> +#ifdef CONFIG_X86_64 >> + bank->mem = ioremap_cache(bank->start, size); >> + if (!bank->mem) >> + return -ENOMEM; >> +#endif >> + >> for (i = 0; i < size; i += PAGE_SIZE) { >> new_epc_page = kzalloc(sizeof(*new_epc_page), GFP_KERNEL); >> if (!new_epc_page) >> goto err_freelist; >> - new_epc_page->pa = start + i; >> + new_epc_page->pa = bank->start + i; >> >> spin_lock(&sgx_free_list_lock); >> list_add_tail(&new_epc_page->free_list, &sgx_free_list); >> @@ -439,10 +453,8 @@ int sgx_page_cache_init(resource_size_t start, unsigned long size) >> spin_unlock(&sgx_free_list_lock); >> } >> >> - sgx_nr_high_pages = 2 * sgx_nr_low_pages; >> - ksgxswapd_tsk = kthread_run(ksgxswapd, NULL, "ksgxswapd"); >> - >> return 0; >> + >> err_freelist: >> list_for_each_safe(parser, temp, &sgx_free_list) { >> spin_lock(&sgx_free_list_lock); >> @@ -451,13 +463,55 @@ int sgx_page_cache_init(resource_size_t start, unsigned long size) >> spin_unlock(&sgx_free_list_lock); >> kfree(entry); >> } >> +#ifdef CONFIG_X86_64 >> + iounmap(bank->mem); >> +#endif >> return -ENOMEM; >> } >> >> -void sgx_page_cache_teardown(void) >> +static void sgx_epc_bank_teardown(struct sgx_epc_bank *bank) >> +{ >> + if (!bank->start || !bank->end) >> + return; >> + >> + /* >> + * Don't free 'struct sgx_epc_page' for EPC page in this bank. All >> + * 'struct sgx_epc_page' for all EPC pages will be destroyed together. >> + */ >> +#ifdef CONFIG_X86_64 >> + if (bank->mem) >> + iounmap(bank->mem); >> +#endif >> +} >> + >> +int sgx_page_cache_init(struct sgx_epc_bank *banks, int nr_banks) >> +{ >> + int i, ret = -ENODEV; >> + >> + for (i = 0; i < nr_banks; i++) { >> + ret = sgx_epc_bank_init(banks + i); >> + if (ret) >> + break; >> + } >> + >> + if (!i) >> + return ret; >> + >> + /* >> + * We can continue to use EPC banks that have been initialized >> + * successfully, even we fail on initializing one particular EPC bank. >> + */ >> + sgx_nr_high_pages = 2 * sgx_nr_low_pages; >> + ksgxswapd_tsk = kthread_run(ksgxswapd, NULL, "ksgxswapd"); >> + >> + return 0; >> +} >> + >> +void sgx_page_cache_teardown(struct sgx_epc_bank *banks, int nr_banks) >> { >> struct sgx_epc_page *entry; >> struct list_head *parser, *temp; >> + int i; >> >> if (ksgxswapd_tsk) >> kthread_stop(ksgxswapd_tsk); >> @@ -469,6 +523,9 @@ void sgx_page_cache_teardown(void) >> kfree(entry); >> } >> spin_unlock(&sgx_free_list_lock); >> + >> + for (i = 0; i < nr_banks; i++) >> + sgx_epc_bank_teardown(banks + i); >> } >> >> static struct sgx_epc_page *sgx_alloc_page_fast(void) >> -- >> 2.9.3 >> > _______________________________________________ > intel-sgx-kernel-dev mailing list > intel-sgx-kernel-dev@lists.01.org > https://lists.01.org/mailman/listinfo/intel-sgx-kernel-dev >
diff --git a/drivers/platform/x86/intel_sgx.h b/drivers/platform/x86/intel_sgx.h index adb5b17..a98915a 100644 --- a/drivers/platform/x86/intel_sgx.h +++ b/drivers/platform/x86/intel_sgx.h @@ -252,8 +252,8 @@ enum sgx_free_flags { }; int ksgxswapd(void *p); -int sgx_page_cache_init(resource_size_t start, unsigned long size); -void sgx_page_cache_teardown(void); +int sgx_page_cache_init(struct sgx_epc_bank *banks, int nr_banks); +void sgx_page_cache_teardown(struct sgx_epc_bank *banks, int nr_banks); struct sgx_epc_page *sgx_alloc_page(struct sgx_tgid_ctx *tgid_epc_cnt, unsigned int flags); int sgx_free_page(struct sgx_epc_page *entry, struct sgx_encl *encl, diff --git a/drivers/platform/x86/intel_sgx_main.c b/drivers/platform/x86/intel_sgx_main.c index 6c2d240..e242969 100644 --- a/drivers/platform/x86/intel_sgx_main.c +++ b/drivers/platform/x86/intel_sgx_main.c @@ -264,7 +264,6 @@ static int sgx_dev_init(struct device *dev) { unsigned int wq_flags; int ret; - int i; pr_info("intel_sgx: " DRV_DESCRIPTION " v" DRV_VERSION "\n"); @@ -277,25 +276,9 @@ static int sgx_dev_init(struct device *dev) pr_info("intel_sgx: Number of EPCs %d\n", sgx_nr_epc_banks); - for (i = 0; i < sgx_nr_epc_banks; i++) { - pr_info("intel_sgx: EPC memory range 0x%lx-0x%lx\n", - sgx_epc_banks[i].start, sgx_epc_banks[i].end); -#ifdef CONFIG_X86_64 - sgx_epc_banks[i].mem = ioremap_cache(sgx_epc_banks[i].start, - sgx_epc_banks[i].end - sgx_epc_banks[i].start); - if (!sgx_epc_banks[i].mem) { - sgx_nr_epc_banks = i; - ret = -ENOMEM; - goto out_iounmap; - } -#endif - ret = sgx_page_cache_init(sgx_epc_banks[i].start, - sgx_epc_banks[i].end - sgx_epc_banks[i].start); - if (ret) { - sgx_nr_epc_banks = i+1; - goto out_iounmap; - } - } + ret = sgx_page_cache_init(sgx_epc_banks, sgx_nr_epc_banks); + if (ret) + return ret; wq_flags = WQ_UNBOUND | WQ_FREEZABLE; #ifdef WQ_NON_REENETRANT @@ -305,7 +288,7 @@ static int sgx_dev_init(struct device *dev) if (!sgx_add_page_wq) { pr_err("intel_sgx: alloc_workqueue() failed\n"); ret = -ENOMEM; - goto out_iounmap; + goto out_page_cache_init; } sgx_dev.parent = dev; @@ -318,11 +301,8 @@ static int sgx_dev_init(struct device *dev) return 0; out_workqueue: destroy_workqueue(sgx_add_page_wq); -out_iounmap: -#ifdef CONFIG_X86_64 - for (i = 0; i < sgx_nr_epc_banks; i++) - iounmap(sgx_epc_banks[i].mem); -#endif +out_page_cache_init: + sgx_page_cache_teardown(sgx_epc_banks, sgx_nr_epc_banks); return ret; } @@ -378,15 +358,9 @@ static int sgx_drv_probe(struct platform_device *pdev) static int sgx_drv_remove(struct platform_device *pdev) { - int i; - misc_deregister(&sgx_dev); destroy_workqueue(sgx_add_page_wq); -#ifdef CONFIG_X86_64 - for (i = 0; i < sgx_nr_epc_banks; i++) - iounmap(sgx_epc_banks[i].mem); -#endif - sgx_page_cache_teardown(); + sgx_page_cache_teardown(sgx_epc_banks, sgx_nr_epc_banks); return 0; } diff --git a/drivers/platform/x86/intel_sgx_page_cache.c b/drivers/platform/x86/intel_sgx_page_cache.c index 852066c..2a277ba 100644 --- a/drivers/platform/x86/intel_sgx_page_cache.c +++ b/drivers/platform/x86/intel_sgx_page_cache.c @@ -420,17 +420,31 @@ int ksgxswapd(void *p) return 0; } -int sgx_page_cache_init(resource_size_t start, unsigned long size) +static int sgx_epc_bank_init(struct sgx_epc_bank *bank) { - unsigned long i; + unsigned long i, size; struct sgx_epc_page *new_epc_page, *entry; struct list_head *parser, *temp; + pr_info("intel_sgx: EPC memory range 0x%lx-0x%lx\n", + bank->start, bank->end); + + if (!bank->start || !bank->end) + return -ENODEV; + + size = bank->end - bank->start; + +#ifdef CONFIG_X86_64 + bank->mem = ioremap_cache(bank->start, size); + if (!bank->mem) + return -ENOMEM; +#endif + for (i = 0; i < size; i += PAGE_SIZE) { new_epc_page = kzalloc(sizeof(*new_epc_page), GFP_KERNEL); if (!new_epc_page) goto err_freelist; - new_epc_page->pa = start + i; + new_epc_page->pa = bank->start + i; spin_lock(&sgx_free_list_lock); list_add_tail(&new_epc_page->free_list, &sgx_free_list); @@ -439,10 +453,8 @@ int sgx_page_cache_init(resource_size_t start, unsigned long size) spin_unlock(&sgx_free_list_lock); } - sgx_nr_high_pages = 2 * sgx_nr_low_pages; - ksgxswapd_tsk = kthread_run(ksgxswapd, NULL, "ksgxswapd"); - return 0; + err_freelist: list_for_each_safe(parser, temp, &sgx_free_list) { spin_lock(&sgx_free_list_lock); @@ -451,13 +463,55 @@ int sgx_page_cache_init(resource_size_t start, unsigned long size) spin_unlock(&sgx_free_list_lock); kfree(entry); } +#ifdef CONFIG_X86_64 + iounmap(bank->mem); +#endif return -ENOMEM; } -void sgx_page_cache_teardown(void) +static void sgx_epc_bank_teardown(struct sgx_epc_bank *bank) +{ + if (!bank->start || !bank->end) + return; + + /* + * Don't free 'struct sgx_epc_page' for EPC page in this bank. All + * 'struct sgx_epc_page' for all EPC pages will be destroyed together. + */ +#ifdef CONFIG_X86_64 + if (bank->mem) + iounmap(bank->mem); +#endif +} + +int sgx_page_cache_init(struct sgx_epc_bank *banks, int nr_banks) +{ + int i, ret = -ENODEV; + + for (i = 0; i < nr_banks; i++) { + ret = sgx_epc_bank_init(banks + i); + if (ret) + break; + } + + if (!i) + return ret; + + /* + * We can continue to use EPC banks that have been initialized + * successfully, even we fail on initializing one particular EPC bank. + */ + sgx_nr_high_pages = 2 * sgx_nr_low_pages; + ksgxswapd_tsk = kthread_run(ksgxswapd, NULL, "ksgxswapd"); + + return 0; +} + +void sgx_page_cache_teardown(struct sgx_epc_bank *banks, int nr_banks) { struct sgx_epc_page *entry; struct list_head *parser, *temp; + int i; if (ksgxswapd_tsk) kthread_stop(ksgxswapd_tsk); @@ -469,6 +523,9 @@ void sgx_page_cache_teardown(void) kfree(entry); } spin_unlock(&sgx_free_list_lock); + + for (i = 0; i < nr_banks; i++) + sgx_epc_bank_teardown(banks + i); } static struct sgx_epc_page *sgx_alloc_page_fast(void)
Currently sgx_page_cache_init is called for each EPC bank in case of multiple EPC banks, which is wrong because sgx_page_cache_init creates ksgxswapd kernel thread which should only be created once in global. This patch refines page cache initlization code by introducing sgx_epc_bank_init which initializes one EPC bank, and sgx_page_cache_init is changed to take all EPC banks as parameter and initialize them one by one by calling sgx_epc_bank_init. And ksgxswapd will be created once in sgx_page_cache_init. ioremap for EPC bank is also moved to sgx_epc_bank_init. This is more reasonable, as EPC page's virtual address handling is also part of page cache management code. Signed-off-by: Kai Huang <kai.huang@linux.intel.com> --- drivers/platform/x86/intel_sgx.h | 4 +- drivers/platform/x86/intel_sgx_main.c | 40 +++------------- drivers/platform/x86/intel_sgx_page_cache.c | 71 ++++++++++++++++++++++++++--- 3 files changed, 73 insertions(+), 42 deletions(-)