| Message ID | 20170404072938.4800-5-kai.huang@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Apr 04, 2017 at 07:29:35PM +1200, Kai Huang wrote: > Rename sgx_get{put}_epc_page to sgx_kmap{kunmap}_epc_page, and expose them. > For normal memory, get{put}_page increases{decreases} reference count of > 'struct page', and they have nothing to do with map/unmap page to kernel > virtual address. Rename the equivalent for EPC to follow this convention. > > Signed-off-by: Kai Huang <kai.huang@linux.intel.com> Is the renaming necessary or is it just your preference? Why are you doing multiple logical changes in one commit? /Jarkko > --- > arch/x86/include/asm/sgx.h | 2 ++ > drivers/platform/x86/intel_sgx.h | 2 -- > drivers/platform/x86/intel_sgx_ioctl.c | 28 ++++++++++++++-------------- > drivers/platform/x86/intel_sgx_page_cache.c | 20 ++++++++++---------- > drivers/platform/x86/intel_sgx_util.c | 18 ++++++++++-------- > drivers/platform/x86/intel_sgx_vma.c | 12 ++++++------ > 6 files changed, 42 insertions(+), 40 deletions(-) > > diff --git a/arch/x86/include/asm/sgx.h b/arch/x86/include/asm/sgx.h > index 0e3fee1..fe1a2ba 100644 > --- a/arch/x86/include/asm/sgx.h > +++ b/arch/x86/include/asm/sgx.h > @@ -369,5 +369,7 @@ struct sgx_epc_page { > > struct sgx_epc_page *sgx_epc_pfn_to_page(unsigned long pfn); > unsigned long sgx_epc_page_to_pfn(struct sgx_epc_page *entry); > +void *sgx_kmap_epc_page(struct sgx_epc_page *entry); > +void sgx_kunmap_epc_page(void *epc_page_vaddr); > > #endif /* _ASM_X86_SGX_H */ > diff --git a/drivers/platform/x86/intel_sgx.h b/drivers/platform/x86/intel_sgx.h > index da662d0..b9b81d6 100644 > --- a/drivers/platform/x86/intel_sgx.h > +++ b/drivers/platform/x86/intel_sgx.h > @@ -199,8 +199,6 @@ long sgx_compat_ioctl(struct file *filep, unsigned int cmd, unsigned long arg); > > /* Utility functions */ > int sgx_test_and_clear_young(struct sgx_encl_page *page, struct sgx_encl *encl); > -void *sgx_get_epc_page(struct sgx_epc_page *entry); > -void sgx_put_epc_page(void *epc_page_vaddr); > struct page *sgx_get_backing(struct sgx_encl *encl, > struct sgx_encl_page *entry, > bool pcmd); > diff --git a/drivers/platform/x86/intel_sgx_ioctl.c b/drivers/platform/x86/intel_sgx_ioctl.c > index 3fc89ac..a8729c3 100644 > --- a/drivers/platform/x86/intel_sgx_ioctl.c > +++ b/drivers/platform/x86/intel_sgx_ioctl.c > @@ -170,13 +170,13 @@ static int sgx_measure(struct sgx_epc_page *secs_page, > if (!(j & mrmask)) > continue; > > - secs = sgx_get_epc_page(secs_page); > - epc = sgx_get_epc_page(epc_page); > + secs = sgx_kmap_epc_page(secs_page); > + epc = sgx_kmap_epc_page(epc_page); > > ret = __eextend(secs, (void *)((unsigned long)epc + i)); > > - sgx_put_epc_page(epc); > - sgx_put_epc_page(secs); > + sgx_kunmap_epc_page(epc); > + sgx_kunmap_epc_page(secs); > } > > return ret; > @@ -193,15 +193,15 @@ static int sgx_add_page(struct sgx_epc_page *secs_page, > int ret; > > pginfo.srcpge = (unsigned long)kmap_atomic(backing); > - pginfo.secs = (unsigned long)sgx_get_epc_page(secs_page); > - epc_page_vaddr = sgx_get_epc_page(epc_page); > + pginfo.secs = (unsigned long)sgx_kmap_epc_page(secs_page); > + epc_page_vaddr = sgx_kmap_epc_page(epc_page); > > pginfo.linaddr = linaddr; > pginfo.secinfo = (unsigned long)secinfo; > ret = __eadd(&pginfo, epc_page_vaddr); > > - sgx_put_epc_page(epc_page_vaddr); > - sgx_put_epc_page((void *)(unsigned long)pginfo.secs); > + sgx_kunmap_epc_page(epc_page_vaddr); > + sgx_kunmap_epc_page((void *)(unsigned long)pginfo.secs); > kunmap_atomic((void *)(unsigned long)pginfo.srcpge); > > return ret; > @@ -411,7 +411,7 @@ static int sgx_init_page(struct sgx_encl *encl, > return PTR_ERR(epc_page); > } > > - vaddr = sgx_get_epc_page(epc_page); > + vaddr = sgx_kmap_epc_page(epc_page); > if (!vaddr) { > sgx_warn(encl, "kmap of a new VA page failed %d\n", > ret); > @@ -421,7 +421,7 @@ static int sgx_init_page(struct sgx_encl *encl, > } > > ret = __epa(vaddr); > - sgx_put_epc_page(vaddr); > + sgx_kunmap_epc_page(vaddr); > > if (ret) { > sgx_warn(encl, "EPA returned %d\n", ret); > @@ -555,7 +555,7 @@ static long sgx_ioc_enclave_create(struct file *filep, unsigned int cmd, > if (ret) > goto out; > > - secs_vaddr = sgx_get_epc_page(secs_epc); > + secs_vaddr = sgx_kmap_epc_page(secs_epc); > > pginfo.srcpge = (unsigned long)secs; > pginfo.linaddr = 0; > @@ -564,7 +564,7 @@ static long sgx_ioc_enclave_create(struct file *filep, unsigned int cmd, > memset(&secinfo, 0, sizeof(secinfo)); > ret = __ecreate((void *)&pginfo, secs_vaddr); > > - sgx_put_epc_page(secs_vaddr); > + sgx_kunmap_epc_page(secs_vaddr); > > if (ret) { > sgx_dbg(encl, "ECREATE returned %ld\n", ret); > @@ -828,9 +828,9 @@ static int __sgx_encl_init(struct sgx_encl *encl, char *sigstruct, > for (i = 0; i < SGX_EINIT_SLEEP_COUNT; i++) { > for (j = 0; j < SGX_EINIT_SPIN_COUNT; j++) { > mutex_lock(&encl->lock); > - secs_va = sgx_get_epc_page(secs_epc); > + secs_va = sgx_kmap_epc_page(secs_epc); > ret = __einit(sigstruct, einittoken, secs_va); > - sgx_put_epc_page(secs_va); > + sgx_kunmap_epc_page(secs_va); > mutex_unlock(&encl->lock); > if (ret == SGX_UNMASKED_EVENT) > continue; > diff --git a/drivers/platform/x86/intel_sgx_page_cache.c b/drivers/platform/x86/intel_sgx_page_cache.c > index 8d323f4..3f9ba9c 100644 > --- a/drivers/platform/x86/intel_sgx_page_cache.c > +++ b/drivers/platform/x86/intel_sgx_page_cache.c > @@ -225,9 +225,9 @@ static void sgx_eblock(struct sgx_encl *encl, > void *vaddr; > int ret; > > - vaddr = sgx_get_epc_page(epc_page); > + vaddr = sgx_kmap_epc_page(epc_page); > ret = __eblock((unsigned long)vaddr); > - sgx_put_epc_page(vaddr); > + sgx_kunmap_epc_page(vaddr); > > if (ret) { > sgx_crit(encl, "EBLOCK returned %d\n", ret); > @@ -242,9 +242,9 @@ static void sgx_etrack(struct sgx_encl *encl) > void *epc; > int ret; > > - epc = sgx_get_epc_page(encl->secs_page.epc_page); > + epc = sgx_kmap_epc_page(encl->secs_page.epc_page); > ret = __etrack(epc); > - sgx_put_epc_page(epc); > + sgx_kunmap_epc_page(epc); > > if (ret) { > sgx_crit(encl, "ETRACK returned %d\n", ret); > @@ -282,8 +282,8 @@ static int __sgx_ewb(struct sgx_encl *encl, > goto out; > } > > - epc = sgx_get_epc_page(encl_page->epc_page); > - va = sgx_get_epc_page(encl_page->va_page->epc_page); > + epc = sgx_kmap_epc_page(encl_page->epc_page); > + va = sgx_kmap_epc_page(encl_page->va_page->epc_page); > > pginfo.srcpge = (unsigned long)kmap_atomic(backing); > pginfo.pcmd = (unsigned long)kmap_atomic(pcmd) + pcmd_offset; > @@ -294,8 +294,8 @@ static int __sgx_ewb(struct sgx_encl *encl, > kunmap_atomic((void *)(unsigned long)(pginfo.pcmd - pcmd_offset)); > kunmap_atomic((void *)(unsigned long)pginfo.srcpge); > > - sgx_put_epc_page(va); > - sgx_put_epc_page(epc); > + sgx_kunmap_epc_page(va); > + sgx_kunmap_epc_page(epc); > sgx_put_backing(pcmd, true); > > out: > @@ -597,9 +597,9 @@ int sgx_free_page(struct sgx_epc_page *entry, struct sgx_encl *encl, > void *epc; > int ret; > > - epc = sgx_get_epc_page(entry); > + epc = sgx_kmap_epc_page(entry); > ret = __eremove(epc); > - sgx_put_epc_page(epc); > + sgx_kunmap_epc_page(epc); > > if (ret) { > if (flags & SGX_FREE_RETURN_ERROR) > diff --git a/drivers/platform/x86/intel_sgx_util.c b/drivers/platform/x86/intel_sgx_util.c > index 6d36b98..c63193d 100644 > --- a/drivers/platform/x86/intel_sgx_util.c > +++ b/drivers/platform/x86/intel_sgx_util.c > @@ -125,7 +125,7 @@ unsigned long sgx_epc_page_to_pfn(struct sgx_epc_page *entry) > } > EXPORT_SYMBOL_GPL(sgx_epc_page_to_pfn); > > -void *sgx_get_epc_page(struct sgx_epc_page *entry) > +void *sgx_kmap_epc_page(struct sgx_epc_page *entry) > { > #ifdef CONFIG_X86_32 > return kmap_atomic_pfn(sgx_epc_page_to_pfn(entry)); > @@ -138,14 +138,16 @@ void *sgx_get_epc_page(struct sgx_epc_page *entry) > return bank->mem + ((entry - bank->epgtbl) << PAGE_SHIFT); > #endif > } > +EXPORT_SYMBOL_GPL(sgx_kmap_epc_page); > > -void sgx_put_epc_page(void *epc_page_vaddr) > +void sgx_kunmap_epc_page(void *epc_page_vaddr) > { > #ifdef CONFIG_X86_32 > kunmap_atomic(epc_page_vaddr); > #else > #endif > } > +EXPORT_SYMBOL_GPL(sgx_kunmap_epc_page); > > struct page *sgx_get_backing(struct sgx_encl *encl, > struct sgx_encl_page *entry, > @@ -313,10 +315,10 @@ static int sgx_eldu(struct sgx_encl *encl, > } > > if (!is_secs) > - secs_ptr = sgx_get_epc_page(encl->secs_page.epc_page); > + secs_ptr = sgx_kmap_epc_page(encl->secs_page.epc_page); > > - epc_ptr = sgx_get_epc_page(epc_page); > - va_ptr = sgx_get_epc_page(encl_page->va_page->epc_page); > + epc_ptr = sgx_kmap_epc_page(epc_page); > + va_ptr = sgx_kmap_epc_page(encl_page->va_page->epc_page); > pginfo.srcpge = (unsigned long)kmap_atomic(backing); > pginfo.pcmd = (unsigned long)kmap_atomic(pcmd) + pcmd_offset; > pginfo.linaddr = is_secs ? 0 : encl_page->addr; > @@ -333,11 +335,11 @@ static int sgx_eldu(struct sgx_encl *encl, > > kunmap_atomic((void *)(unsigned long)(pginfo.pcmd - pcmd_offset)); > kunmap_atomic((void *)(unsigned long)pginfo.srcpge); > - sgx_put_epc_page(va_ptr); > - sgx_put_epc_page(epc_ptr); > + sgx_kunmap_epc_page(va_ptr); > + sgx_kunmap_epc_page(epc_ptr); > > if (!is_secs) > - sgx_put_epc_page(secs_ptr); > + sgx_kunmap_epc_page(secs_ptr); > > sgx_put_backing(pcmd, false); > > diff --git a/drivers/platform/x86/intel_sgx_vma.c b/drivers/platform/x86/intel_sgx_vma.c > index 854bd7a..d80b22a 100644 > --- a/drivers/platform/x86/intel_sgx_vma.c > +++ b/drivers/platform/x86/intel_sgx_vma.c > @@ -132,10 +132,10 @@ static inline int sgx_vma_access_word(struct sgx_encl *encl, > return -ECANCELED; > > if (align || (cnt != sizeof(unsigned long))) { > - vaddr = sgx_get_epc_page(encl_page->epc_page); > + vaddr = sgx_kmap_epc_page(encl_page->epc_page); > ret = __edbgrd((void *)((unsigned long)vaddr + offset), > (unsigned long *)data); > - sgx_put_epc_page(vaddr); > + sgx_kunmap_epc_page(vaddr); > if (ret) { > sgx_dbg(encl, "EDBGRD returned %d\n", ret); > return -EFAULT; > @@ -143,10 +143,10 @@ static inline int sgx_vma_access_word(struct sgx_encl *encl, > } > > memcpy(data + align, buf + i, cnt); > - vaddr = sgx_get_epc_page(encl_page->epc_page); > + vaddr = sgx_kmap_epc_page(encl_page->epc_page); > ret = __edbgwr((void *)((unsigned long)vaddr + offset), > (unsigned long *)data); > - sgx_put_epc_page(vaddr); > + sgx_kunmap_epc_page(vaddr); > if (ret) { > sgx_dbg(encl, "EDBGWR returned %d\n", ret); > return -EFAULT; > @@ -156,10 +156,10 @@ static inline int sgx_vma_access_word(struct sgx_encl *encl, > (offset + (len - i)) > 72) > return -ECANCELED; > > - vaddr = sgx_get_epc_page(encl_page->epc_page); > + vaddr = sgx_kmap_epc_page(encl_page->epc_page); > ret = __edbgrd((void *)((unsigned long)vaddr + offset), > (unsigned long *)data); > - sgx_put_epc_page(vaddr); > + sgx_kunmap_epc_page(vaddr); > if (ret) { > sgx_dbg(encl, "EDBGRD returned %d\n", ret); > return -EFAULT; > -- > 2.9.3 >
On 4/5/2017 2:51 AM, Jarkko Sakkinen wrote: > On Tue, Apr 04, 2017 at 07:29:35PM +1200, Kai Huang wrote: >> Rename sgx_get{put}_epc_page to sgx_kmap{kunmap}_epc_page, and expose them. >> For normal memory, get{put}_page increases{decreases} reference count of >> 'struct page', and they have nothing to do with map/unmap page to kernel >> virtual address. Rename the equivalent for EPC to follow this convention. >> >> Signed-off-by: Kai Huang <kai.huang@linux.intel.com> > > Is the renaming necessary or is it just your preference? Besides my preference another reason I did the renaming is I used to have sgx_get{put}_epc_page to increase/decrease refcount in 'struct sgx_epc_page', but as I said in reply to patch 3 I recently have removed sgx_get{put}_epc_page, so this is not a concern to me anymore, although I think it's still better to have sgx_get{put}_epc_page to increase/decrease refcount. I can drop this patch if you don't like it. Why are you > doing multiple logical changes in one commit? I really don't understand what do you mean about 'multiple logical changes' here. This patch is only about renaming and exposing the two functions. Thanks, -Kai > > /Jarkko > >> --- >> arch/x86/include/asm/sgx.h | 2 ++ >> drivers/platform/x86/intel_sgx.h | 2 -- >> drivers/platform/x86/intel_sgx_ioctl.c | 28 ++++++++++++++-------------- >> drivers/platform/x86/intel_sgx_page_cache.c | 20 ++++++++++---------- >> drivers/platform/x86/intel_sgx_util.c | 18 ++++++++++-------- >> drivers/platform/x86/intel_sgx_vma.c | 12 ++++++------ >> 6 files changed, 42 insertions(+), 40 deletions(-) >> >> diff --git a/arch/x86/include/asm/sgx.h b/arch/x86/include/asm/sgx.h >> index 0e3fee1..fe1a2ba 100644 >> --- a/arch/x86/include/asm/sgx.h >> +++ b/arch/x86/include/asm/sgx.h >> @@ -369,5 +369,7 @@ struct sgx_epc_page { >> >> struct sgx_epc_page *sgx_epc_pfn_to_page(unsigned long pfn); >> unsigned long sgx_epc_page_to_pfn(struct sgx_epc_page *entry); >> +void *sgx_kmap_epc_page(struct sgx_epc_page *entry); >> +void sgx_kunmap_epc_page(void *epc_page_vaddr); >> >> #endif /* _ASM_X86_SGX_H */ >> diff --git a/drivers/platform/x86/intel_sgx.h b/drivers/platform/x86/intel_sgx.h >> index da662d0..b9b81d6 100644 >> --- a/drivers/platform/x86/intel_sgx.h >> +++ b/drivers/platform/x86/intel_sgx.h >> @@ -199,8 +199,6 @@ long sgx_compat_ioctl(struct file *filep, unsigned int cmd, unsigned long arg); >> >> /* Utility functions */ >> int sgx_test_and_clear_young(struct sgx_encl_page *page, struct sgx_encl *encl); >> -void *sgx_get_epc_page(struct sgx_epc_page *entry); >> -void sgx_put_epc_page(void *epc_page_vaddr); >> struct page *sgx_get_backing(struct sgx_encl *encl, >> struct sgx_encl_page *entry, >> bool pcmd); >> diff --git a/drivers/platform/x86/intel_sgx_ioctl.c b/drivers/platform/x86/intel_sgx_ioctl.c >> index 3fc89ac..a8729c3 100644 >> --- a/drivers/platform/x86/intel_sgx_ioctl.c >> +++ b/drivers/platform/x86/intel_sgx_ioctl.c >> @@ -170,13 +170,13 @@ static int sgx_measure(struct sgx_epc_page *secs_page, >> if (!(j & mrmask)) >> continue; >> >> - secs = sgx_get_epc_page(secs_page); >> - epc = sgx_get_epc_page(epc_page); >> + secs = sgx_kmap_epc_page(secs_page); >> + epc = sgx_kmap_epc_page(epc_page); >> >> ret = __eextend(secs, (void *)((unsigned long)epc + i)); >> >> - sgx_put_epc_page(epc); >> - sgx_put_epc_page(secs); >> + sgx_kunmap_epc_page(epc); >> + sgx_kunmap_epc_page(secs); >> } >> >> return ret; >> @@ -193,15 +193,15 @@ static int sgx_add_page(struct sgx_epc_page *secs_page, >> int ret; >> >> pginfo.srcpge = (unsigned long)kmap_atomic(backing); >> - pginfo.secs = (unsigned long)sgx_get_epc_page(secs_page); >> - epc_page_vaddr = sgx_get_epc_page(epc_page); >> + pginfo.secs = (unsigned long)sgx_kmap_epc_page(secs_page); >> + epc_page_vaddr = sgx_kmap_epc_page(epc_page); >> >> pginfo.linaddr = linaddr; >> pginfo.secinfo = (unsigned long)secinfo; >> ret = __eadd(&pginfo, epc_page_vaddr); >> >> - sgx_put_epc_page(epc_page_vaddr); >> - sgx_put_epc_page((void *)(unsigned long)pginfo.secs); >> + sgx_kunmap_epc_page(epc_page_vaddr); >> + sgx_kunmap_epc_page((void *)(unsigned long)pginfo.secs); >> kunmap_atomic((void *)(unsigned long)pginfo.srcpge); >> >> return ret; >> @@ -411,7 +411,7 @@ static int sgx_init_page(struct sgx_encl *encl, >> return PTR_ERR(epc_page); >> } >> >> - vaddr = sgx_get_epc_page(epc_page); >> + vaddr = sgx_kmap_epc_page(epc_page); >> if (!vaddr) { >> sgx_warn(encl, "kmap of a new VA page failed %d\n", >> ret); >> @@ -421,7 +421,7 @@ static int sgx_init_page(struct sgx_encl *encl, >> } >> >> ret = __epa(vaddr); >> - sgx_put_epc_page(vaddr); >> + sgx_kunmap_epc_page(vaddr); >> >> if (ret) { >> sgx_warn(encl, "EPA returned %d\n", ret); >> @@ -555,7 +555,7 @@ static long sgx_ioc_enclave_create(struct file *filep, unsigned int cmd, >> if (ret) >> goto out; >> >> - secs_vaddr = sgx_get_epc_page(secs_epc); >> + secs_vaddr = sgx_kmap_epc_page(secs_epc); >> >> pginfo.srcpge = (unsigned long)secs; >> pginfo.linaddr = 0; >> @@ -564,7 +564,7 @@ static long sgx_ioc_enclave_create(struct file *filep, unsigned int cmd, >> memset(&secinfo, 0, sizeof(secinfo)); >> ret = __ecreate((void *)&pginfo, secs_vaddr); >> >> - sgx_put_epc_page(secs_vaddr); >> + sgx_kunmap_epc_page(secs_vaddr); >> >> if (ret) { >> sgx_dbg(encl, "ECREATE returned %ld\n", ret); >> @@ -828,9 +828,9 @@ static int __sgx_encl_init(struct sgx_encl *encl, char *sigstruct, >> for (i = 0; i < SGX_EINIT_SLEEP_COUNT; i++) { >> for (j = 0; j < SGX_EINIT_SPIN_COUNT; j++) { >> mutex_lock(&encl->lock); >> - secs_va = sgx_get_epc_page(secs_epc); >> + secs_va = sgx_kmap_epc_page(secs_epc); >> ret = __einit(sigstruct, einittoken, secs_va); >> - sgx_put_epc_page(secs_va); >> + sgx_kunmap_epc_page(secs_va); >> mutex_unlock(&encl->lock); >> if (ret == SGX_UNMASKED_EVENT) >> continue; >> diff --git a/drivers/platform/x86/intel_sgx_page_cache.c b/drivers/platform/x86/intel_sgx_page_cache.c >> index 8d323f4..3f9ba9c 100644 >> --- a/drivers/platform/x86/intel_sgx_page_cache.c >> +++ b/drivers/platform/x86/intel_sgx_page_cache.c >> @@ -225,9 +225,9 @@ static void sgx_eblock(struct sgx_encl *encl, >> void *vaddr; >> int ret; >> >> - vaddr = sgx_get_epc_page(epc_page); >> + vaddr = sgx_kmap_epc_page(epc_page); >> ret = __eblock((unsigned long)vaddr); >> - sgx_put_epc_page(vaddr); >> + sgx_kunmap_epc_page(vaddr); >> >> if (ret) { >> sgx_crit(encl, "EBLOCK returned %d\n", ret); >> @@ -242,9 +242,9 @@ static void sgx_etrack(struct sgx_encl *encl) >> void *epc; >> int ret; >> >> - epc = sgx_get_epc_page(encl->secs_page.epc_page); >> + epc = sgx_kmap_epc_page(encl->secs_page.epc_page); >> ret = __etrack(epc); >> - sgx_put_epc_page(epc); >> + sgx_kunmap_epc_page(epc); >> >> if (ret) { >> sgx_crit(encl, "ETRACK returned %d\n", ret); >> @@ -282,8 +282,8 @@ static int __sgx_ewb(struct sgx_encl *encl, >> goto out; >> } >> >> - epc = sgx_get_epc_page(encl_page->epc_page); >> - va = sgx_get_epc_page(encl_page->va_page->epc_page); >> + epc = sgx_kmap_epc_page(encl_page->epc_page); >> + va = sgx_kmap_epc_page(encl_page->va_page->epc_page); >> >> pginfo.srcpge = (unsigned long)kmap_atomic(backing); >> pginfo.pcmd = (unsigned long)kmap_atomic(pcmd) + pcmd_offset; >> @@ -294,8 +294,8 @@ static int __sgx_ewb(struct sgx_encl *encl, >> kunmap_atomic((void *)(unsigned long)(pginfo.pcmd - pcmd_offset)); >> kunmap_atomic((void *)(unsigned long)pginfo.srcpge); >> >> - sgx_put_epc_page(va); >> - sgx_put_epc_page(epc); >> + sgx_kunmap_epc_page(va); >> + sgx_kunmap_epc_page(epc); >> sgx_put_backing(pcmd, true); >> >> out: >> @@ -597,9 +597,9 @@ int sgx_free_page(struct sgx_epc_page *entry, struct sgx_encl *encl, >> void *epc; >> int ret; >> >> - epc = sgx_get_epc_page(entry); >> + epc = sgx_kmap_epc_page(entry); >> ret = __eremove(epc); >> - sgx_put_epc_page(epc); >> + sgx_kunmap_epc_page(epc); >> >> if (ret) { >> if (flags & SGX_FREE_RETURN_ERROR) >> diff --git a/drivers/platform/x86/intel_sgx_util.c b/drivers/platform/x86/intel_sgx_util.c >> index 6d36b98..c63193d 100644 >> --- a/drivers/platform/x86/intel_sgx_util.c >> +++ b/drivers/platform/x86/intel_sgx_util.c >> @@ -125,7 +125,7 @@ unsigned long sgx_epc_page_to_pfn(struct sgx_epc_page *entry) >> } >> EXPORT_SYMBOL_GPL(sgx_epc_page_to_pfn); >> >> -void *sgx_get_epc_page(struct sgx_epc_page *entry) >> +void *sgx_kmap_epc_page(struct sgx_epc_page *entry) >> { >> #ifdef CONFIG_X86_32 >> return kmap_atomic_pfn(sgx_epc_page_to_pfn(entry)); >> @@ -138,14 +138,16 @@ void *sgx_get_epc_page(struct sgx_epc_page *entry) >> return bank->mem + ((entry - bank->epgtbl) << PAGE_SHIFT); >> #endif >> } >> +EXPORT_SYMBOL_GPL(sgx_kmap_epc_page); >> >> -void sgx_put_epc_page(void *epc_page_vaddr) >> +void sgx_kunmap_epc_page(void *epc_page_vaddr) >> { >> #ifdef CONFIG_X86_32 >> kunmap_atomic(epc_page_vaddr); >> #else >> #endif >> } >> +EXPORT_SYMBOL_GPL(sgx_kunmap_epc_page); >> >> struct page *sgx_get_backing(struct sgx_encl *encl, >> struct sgx_encl_page *entry, >> @@ -313,10 +315,10 @@ static int sgx_eldu(struct sgx_encl *encl, >> } >> >> if (!is_secs) >> - secs_ptr = sgx_get_epc_page(encl->secs_page.epc_page); >> + secs_ptr = sgx_kmap_epc_page(encl->secs_page.epc_page); >> >> - epc_ptr = sgx_get_epc_page(epc_page); >> - va_ptr = sgx_get_epc_page(encl_page->va_page->epc_page); >> + epc_ptr = sgx_kmap_epc_page(epc_page); >> + va_ptr = sgx_kmap_epc_page(encl_page->va_page->epc_page); >> pginfo.srcpge = (unsigned long)kmap_atomic(backing); >> pginfo.pcmd = (unsigned long)kmap_atomic(pcmd) + pcmd_offset; >> pginfo.linaddr = is_secs ? 0 : encl_page->addr; >> @@ -333,11 +335,11 @@ static int sgx_eldu(struct sgx_encl *encl, >> >> kunmap_atomic((void *)(unsigned long)(pginfo.pcmd - pcmd_offset)); >> kunmap_atomic((void *)(unsigned long)pginfo.srcpge); >> - sgx_put_epc_page(va_ptr); >> - sgx_put_epc_page(epc_ptr); >> + sgx_kunmap_epc_page(va_ptr); >> + sgx_kunmap_epc_page(epc_ptr); >> >> if (!is_secs) >> - sgx_put_epc_page(secs_ptr); >> + sgx_kunmap_epc_page(secs_ptr); >> >> sgx_put_backing(pcmd, false); >> >> diff --git a/drivers/platform/x86/intel_sgx_vma.c b/drivers/platform/x86/intel_sgx_vma.c >> index 854bd7a..d80b22a 100644 >> --- a/drivers/platform/x86/intel_sgx_vma.c >> +++ b/drivers/platform/x86/intel_sgx_vma.c >> @@ -132,10 +132,10 @@ static inline int sgx_vma_access_word(struct sgx_encl *encl, >> return -ECANCELED; >> >> if (align || (cnt != sizeof(unsigned long))) { >> - vaddr = sgx_get_epc_page(encl_page->epc_page); >> + vaddr = sgx_kmap_epc_page(encl_page->epc_page); >> ret = __edbgrd((void *)((unsigned long)vaddr + offset), >> (unsigned long *)data); >> - sgx_put_epc_page(vaddr); >> + sgx_kunmap_epc_page(vaddr); >> if (ret) { >> sgx_dbg(encl, "EDBGRD returned %d\n", ret); >> return -EFAULT; >> @@ -143,10 +143,10 @@ static inline int sgx_vma_access_word(struct sgx_encl *encl, >> } >> >> memcpy(data + align, buf + i, cnt); >> - vaddr = sgx_get_epc_page(encl_page->epc_page); >> + vaddr = sgx_kmap_epc_page(encl_page->epc_page); >> ret = __edbgwr((void *)((unsigned long)vaddr + offset), >> (unsigned long *)data); >> - sgx_put_epc_page(vaddr); >> + sgx_kunmap_epc_page(vaddr); >> if (ret) { >> sgx_dbg(encl, "EDBGWR returned %d\n", ret); >> return -EFAULT; >> @@ -156,10 +156,10 @@ static inline int sgx_vma_access_word(struct sgx_encl *encl, >> (offset + (len - i)) > 72) >> return -ECANCELED; >> >> - vaddr = sgx_get_epc_page(encl_page->epc_page); >> + vaddr = sgx_kmap_epc_page(encl_page->epc_page); >> ret = __edbgrd((void *)((unsigned long)vaddr + offset), >> (unsigned long *)data); >> - sgx_put_epc_page(vaddr); >> + sgx_kunmap_epc_page(vaddr); >> if (ret) { >> sgx_dbg(encl, "EDBGRD returned %d\n", ret); >> return -EFAULT; >> -- >> 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 >
On Wed, Apr 05, 2017 at 12:16:46PM +1200, Huang, Kai wrote: > > On 4/5/2017 2:51 AM, Jarkko Sakkinen wrote: > > On Tue, Apr 04, 2017 at 07:29:35PM +1200, Kai Huang wrote: > > > Rename sgx_get{put}_epc_page to sgx_kmap{kunmap}_epc_page, and expose them. > > > For normal memory, get{put}_page increases{decreases} reference count of > > > 'struct page', and they have nothing to do with map/unmap page to kernel > > > virtual address. Rename the equivalent for EPC to follow this convention. > > > > > > Signed-off-by: Kai Huang <kai.huang@linux.intel.com> > > > > Is the renaming necessary or is it just your preference? > > Besides my preference another reason I did the renaming is I used to have > sgx_get{put}_epc_page to increase/decrease refcount in 'struct > sgx_epc_page', but as I said in reply to patch 3 I recently have removed > sgx_get{put}_epc_page, so this is not a concern to me anymore, although I > think it's still better to have sgx_get{put}_epc_page to increase/decrease > refcount. I can drop this patch if you don't like it. > > Why are you > > doing multiple logical changes in one commit? > > I really don't understand what do you mean about 'multiple logical changes' > here. This patch is only about renaming and exposing the two functions. > > Thanks, > -Kai Maybe this will make more sense when it is bundled with virtualization patches. Right now I don't really understand it. /Jarkko
On 4/5/2017 7:38 PM, Jarkko Sakkinen wrote: > On Wed, Apr 05, 2017 at 12:16:46PM +1200, Huang, Kai wrote: >> >> On 4/5/2017 2:51 AM, Jarkko Sakkinen wrote: >>> On Tue, Apr 04, 2017 at 07:29:35PM +1200, Kai Huang wrote: >>>> Rename sgx_get{put}_epc_page to sgx_kmap{kunmap}_epc_page, and expose them. >>>> For normal memory, get{put}_page increases{decreases} reference count of >>>> 'struct page', and they have nothing to do with map/unmap page to kernel >>>> virtual address. Rename the equivalent for EPC to follow this convention. >>>> >>>> Signed-off-by: Kai Huang <kai.huang@linux.intel.com> >>> >>> Is the renaming necessary or is it just your preference? >> >> Besides my preference another reason I did the renaming is I used to have >> sgx_get{put}_epc_page to increase/decrease refcount in 'struct >> sgx_epc_page', but as I said in reply to patch 3 I recently have removed >> sgx_get{put}_epc_page, so this is not a concern to me anymore, although I >> think it's still better to have sgx_get{put}_epc_page to increase/decrease >> refcount. I can drop this patch if you don't like it. >> >> Why are you >>> doing multiple logical changes in one commit? >> >> I really don't understand what do you mean about 'multiple logical changes' >> here. This patch is only about renaming and exposing the two functions. >> >> Thanks, >> -Kai > > Maybe this will make more sense when it is bundled with virtualization > patches. Right now I don't really understand it. I am planning to send out RFC patches of KVM once I finish the unified oversubscription. In fact the purpose of this patch is pretty simple -- besides sgx_alloc{free}_page, KVM also needs sgx_get{put}_epc_page to get/put (or map/unmap) the kernel virtual address of that EPC page for ENCLS to run, just as the same usage in SGX driver. Therefore the main purpose is to expose the two functions (which is mandatory), but I also did the renaming simply because I thought sgx_kmap{kunmap} are better names. Hope I made this clear. Thanks, -Kai > > /Jarkko >
On Thu, Apr 06, 2017 at 09:00:37PM +1200, Huang, Kai wrote: > > > On 4/5/2017 7:38 PM, Jarkko Sakkinen wrote: > > On Wed, Apr 05, 2017 at 12:16:46PM +1200, Huang, Kai wrote: > > > > > > On 4/5/2017 2:51 AM, Jarkko Sakkinen wrote: > > > > On Tue, Apr 04, 2017 at 07:29:35PM +1200, Kai Huang wrote: > > > > > Rename sgx_get{put}_epc_page to sgx_kmap{kunmap}_epc_page, and expose them. > > > > > For normal memory, get{put}_page increases{decreases} reference count of > > > > > 'struct page', and they have nothing to do with map/unmap page to kernel > > > > > virtual address. Rename the equivalent for EPC to follow this convention. > > > > > > > > > > Signed-off-by: Kai Huang <kai.huang@linux.intel.com> > > > > > > > > Is the renaming necessary or is it just your preference? > > > > > > Besides my preference another reason I did the renaming is I used to have > > > sgx_get{put}_epc_page to increase/decrease refcount in 'struct > > > sgx_epc_page', but as I said in reply to patch 3 I recently have removed > > > sgx_get{put}_epc_page, so this is not a concern to me anymore, although I > > > think it's still better to have sgx_get{put}_epc_page to increase/decrease > > > refcount. I can drop this patch if you don't like it. > > > > > > Why are you > > > > doing multiple logical changes in one commit? > > > > > > I really don't understand what do you mean about 'multiple logical changes' > > > here. This patch is only about renaming and exposing the two functions. > > > > > > Thanks, > > > -Kai > > > > Maybe this will make more sense when it is bundled with virtualization > > patches. Right now I don't really understand it. > > I am planning to send out RFC patches of KVM once I finish the unified > oversubscription. > > In fact the purpose of this patch is pretty simple -- besides > sgx_alloc{free}_page, KVM also needs sgx_get{put}_epc_page to get/put (or > map/unmap) the kernel virtual address of that EPC page for ENCLS to run, > just as the same usage in SGX driver. Therefore the main purpose is to > expose the two functions (which is mandatory), but I also did the renaming > simply because I thought sgx_kmap{kunmap} are better names. Hope I made this > clear. > > Thanks, > -Kai Maybe you could keep this part of that patch set (for now)? /Jarkko
On 4/7/2017 6:46 AM, Jarkko Sakkinen wrote: > On Thu, Apr 06, 2017 at 09:00:37PM +1200, Huang, Kai wrote: >> >> >> On 4/5/2017 7:38 PM, Jarkko Sakkinen wrote: >>> On Wed, Apr 05, 2017 at 12:16:46PM +1200, Huang, Kai wrote: >>>> >>>> On 4/5/2017 2:51 AM, Jarkko Sakkinen wrote: >>>>> On Tue, Apr 04, 2017 at 07:29:35PM +1200, Kai Huang wrote: >>>>>> Rename sgx_get{put}_epc_page to sgx_kmap{kunmap}_epc_page, and expose them. >>>>>> For normal memory, get{put}_page increases{decreases} reference count of >>>>>> 'struct page', and they have nothing to do with map/unmap page to kernel >>>>>> virtual address. Rename the equivalent for EPC to follow this convention. >>>>>> >>>>>> Signed-off-by: Kai Huang <kai.huang@linux.intel.com> >>>>> >>>>> Is the renaming necessary or is it just your preference? >>>> >>>> Besides my preference another reason I did the renaming is I used to have >>>> sgx_get{put}_epc_page to increase/decrease refcount in 'struct >>>> sgx_epc_page', but as I said in reply to patch 3 I recently have removed >>>> sgx_get{put}_epc_page, so this is not a concern to me anymore, although I >>>> think it's still better to have sgx_get{put}_epc_page to increase/decrease >>>> refcount. I can drop this patch if you don't like it. >>>> >>>> Why are you >>>>> doing multiple logical changes in one commit? >>>> >>>> I really don't understand what do you mean about 'multiple logical changes' >>>> here. This patch is only about renaming and exposing the two functions. >>>> >>>> Thanks, >>>> -Kai >>> >>> Maybe this will make more sense when it is bundled with virtualization >>> patches. Right now I don't really understand it. >> >> I am planning to send out RFC patches of KVM once I finish the unified >> oversubscription. >> >> In fact the purpose of this patch is pretty simple -- besides >> sgx_alloc{free}_page, KVM also needs sgx_get{put}_epc_page to get/put (or >> map/unmap) the kernel virtual address of that EPC page for ENCLS to run, >> just as the same usage in SGX driver. Therefore the main purpose is to >> expose the two functions (which is mandatory), but I also did the renaming >> simply because I thought sgx_kmap{kunmap} are better names. Hope I made this >> clear. >> >> Thanks, >> -Kai > > Maybe you could keep this part of that patch set (for now)? Part of KVM SGX patch set? No problem. Actually as I said I plan to drop this patch and use driver's sgx_get{put}_epc_page. Thanks, -Kai > > /Jarkko >
diff --git a/arch/x86/include/asm/sgx.h b/arch/x86/include/asm/sgx.h index 0e3fee1..fe1a2ba 100644 --- a/arch/x86/include/asm/sgx.h +++ b/arch/x86/include/asm/sgx.h @@ -369,5 +369,7 @@ struct sgx_epc_page { struct sgx_epc_page *sgx_epc_pfn_to_page(unsigned long pfn); unsigned long sgx_epc_page_to_pfn(struct sgx_epc_page *entry); +void *sgx_kmap_epc_page(struct sgx_epc_page *entry); +void sgx_kunmap_epc_page(void *epc_page_vaddr); #endif /* _ASM_X86_SGX_H */ diff --git a/drivers/platform/x86/intel_sgx.h b/drivers/platform/x86/intel_sgx.h index da662d0..b9b81d6 100644 --- a/drivers/platform/x86/intel_sgx.h +++ b/drivers/platform/x86/intel_sgx.h @@ -199,8 +199,6 @@ long sgx_compat_ioctl(struct file *filep, unsigned int cmd, unsigned long arg); /* Utility functions */ int sgx_test_and_clear_young(struct sgx_encl_page *page, struct sgx_encl *encl); -void *sgx_get_epc_page(struct sgx_epc_page *entry); -void sgx_put_epc_page(void *epc_page_vaddr); struct page *sgx_get_backing(struct sgx_encl *encl, struct sgx_encl_page *entry, bool pcmd); diff --git a/drivers/platform/x86/intel_sgx_ioctl.c b/drivers/platform/x86/intel_sgx_ioctl.c index 3fc89ac..a8729c3 100644 --- a/drivers/platform/x86/intel_sgx_ioctl.c +++ b/drivers/platform/x86/intel_sgx_ioctl.c @@ -170,13 +170,13 @@ static int sgx_measure(struct sgx_epc_page *secs_page, if (!(j & mrmask)) continue; - secs = sgx_get_epc_page(secs_page); - epc = sgx_get_epc_page(epc_page); + secs = sgx_kmap_epc_page(secs_page); + epc = sgx_kmap_epc_page(epc_page); ret = __eextend(secs, (void *)((unsigned long)epc + i)); - sgx_put_epc_page(epc); - sgx_put_epc_page(secs); + sgx_kunmap_epc_page(epc); + sgx_kunmap_epc_page(secs); } return ret; @@ -193,15 +193,15 @@ static int sgx_add_page(struct sgx_epc_page *secs_page, int ret; pginfo.srcpge = (unsigned long)kmap_atomic(backing); - pginfo.secs = (unsigned long)sgx_get_epc_page(secs_page); - epc_page_vaddr = sgx_get_epc_page(epc_page); + pginfo.secs = (unsigned long)sgx_kmap_epc_page(secs_page); + epc_page_vaddr = sgx_kmap_epc_page(epc_page); pginfo.linaddr = linaddr; pginfo.secinfo = (unsigned long)secinfo; ret = __eadd(&pginfo, epc_page_vaddr); - sgx_put_epc_page(epc_page_vaddr); - sgx_put_epc_page((void *)(unsigned long)pginfo.secs); + sgx_kunmap_epc_page(epc_page_vaddr); + sgx_kunmap_epc_page((void *)(unsigned long)pginfo.secs); kunmap_atomic((void *)(unsigned long)pginfo.srcpge); return ret; @@ -411,7 +411,7 @@ static int sgx_init_page(struct sgx_encl *encl, return PTR_ERR(epc_page); } - vaddr = sgx_get_epc_page(epc_page); + vaddr = sgx_kmap_epc_page(epc_page); if (!vaddr) { sgx_warn(encl, "kmap of a new VA page failed %d\n", ret); @@ -421,7 +421,7 @@ static int sgx_init_page(struct sgx_encl *encl, } ret = __epa(vaddr); - sgx_put_epc_page(vaddr); + sgx_kunmap_epc_page(vaddr); if (ret) { sgx_warn(encl, "EPA returned %d\n", ret); @@ -555,7 +555,7 @@ static long sgx_ioc_enclave_create(struct file *filep, unsigned int cmd, if (ret) goto out; - secs_vaddr = sgx_get_epc_page(secs_epc); + secs_vaddr = sgx_kmap_epc_page(secs_epc); pginfo.srcpge = (unsigned long)secs; pginfo.linaddr = 0; @@ -564,7 +564,7 @@ static long sgx_ioc_enclave_create(struct file *filep, unsigned int cmd, memset(&secinfo, 0, sizeof(secinfo)); ret = __ecreate((void *)&pginfo, secs_vaddr); - sgx_put_epc_page(secs_vaddr); + sgx_kunmap_epc_page(secs_vaddr); if (ret) { sgx_dbg(encl, "ECREATE returned %ld\n", ret); @@ -828,9 +828,9 @@ static int __sgx_encl_init(struct sgx_encl *encl, char *sigstruct, for (i = 0; i < SGX_EINIT_SLEEP_COUNT; i++) { for (j = 0; j < SGX_EINIT_SPIN_COUNT; j++) { mutex_lock(&encl->lock); - secs_va = sgx_get_epc_page(secs_epc); + secs_va = sgx_kmap_epc_page(secs_epc); ret = __einit(sigstruct, einittoken, secs_va); - sgx_put_epc_page(secs_va); + sgx_kunmap_epc_page(secs_va); mutex_unlock(&encl->lock); if (ret == SGX_UNMASKED_EVENT) continue; diff --git a/drivers/platform/x86/intel_sgx_page_cache.c b/drivers/platform/x86/intel_sgx_page_cache.c index 8d323f4..3f9ba9c 100644 --- a/drivers/platform/x86/intel_sgx_page_cache.c +++ b/drivers/platform/x86/intel_sgx_page_cache.c @@ -225,9 +225,9 @@ static void sgx_eblock(struct sgx_encl *encl, void *vaddr; int ret; - vaddr = sgx_get_epc_page(epc_page); + vaddr = sgx_kmap_epc_page(epc_page); ret = __eblock((unsigned long)vaddr); - sgx_put_epc_page(vaddr); + sgx_kunmap_epc_page(vaddr); if (ret) { sgx_crit(encl, "EBLOCK returned %d\n", ret); @@ -242,9 +242,9 @@ static void sgx_etrack(struct sgx_encl *encl) void *epc; int ret; - epc = sgx_get_epc_page(encl->secs_page.epc_page); + epc = sgx_kmap_epc_page(encl->secs_page.epc_page); ret = __etrack(epc); - sgx_put_epc_page(epc); + sgx_kunmap_epc_page(epc); if (ret) { sgx_crit(encl, "ETRACK returned %d\n", ret); @@ -282,8 +282,8 @@ static int __sgx_ewb(struct sgx_encl *encl, goto out; } - epc = sgx_get_epc_page(encl_page->epc_page); - va = sgx_get_epc_page(encl_page->va_page->epc_page); + epc = sgx_kmap_epc_page(encl_page->epc_page); + va = sgx_kmap_epc_page(encl_page->va_page->epc_page); pginfo.srcpge = (unsigned long)kmap_atomic(backing); pginfo.pcmd = (unsigned long)kmap_atomic(pcmd) + pcmd_offset; @@ -294,8 +294,8 @@ static int __sgx_ewb(struct sgx_encl *encl, kunmap_atomic((void *)(unsigned long)(pginfo.pcmd - pcmd_offset)); kunmap_atomic((void *)(unsigned long)pginfo.srcpge); - sgx_put_epc_page(va); - sgx_put_epc_page(epc); + sgx_kunmap_epc_page(va); + sgx_kunmap_epc_page(epc); sgx_put_backing(pcmd, true); out: @@ -597,9 +597,9 @@ int sgx_free_page(struct sgx_epc_page *entry, struct sgx_encl *encl, void *epc; int ret; - epc = sgx_get_epc_page(entry); + epc = sgx_kmap_epc_page(entry); ret = __eremove(epc); - sgx_put_epc_page(epc); + sgx_kunmap_epc_page(epc); if (ret) { if (flags & SGX_FREE_RETURN_ERROR) diff --git a/drivers/platform/x86/intel_sgx_util.c b/drivers/platform/x86/intel_sgx_util.c index 6d36b98..c63193d 100644 --- a/drivers/platform/x86/intel_sgx_util.c +++ b/drivers/platform/x86/intel_sgx_util.c @@ -125,7 +125,7 @@ unsigned long sgx_epc_page_to_pfn(struct sgx_epc_page *entry) } EXPORT_SYMBOL_GPL(sgx_epc_page_to_pfn); -void *sgx_get_epc_page(struct sgx_epc_page *entry) +void *sgx_kmap_epc_page(struct sgx_epc_page *entry) { #ifdef CONFIG_X86_32 return kmap_atomic_pfn(sgx_epc_page_to_pfn(entry)); @@ -138,14 +138,16 @@ void *sgx_get_epc_page(struct sgx_epc_page *entry) return bank->mem + ((entry - bank->epgtbl) << PAGE_SHIFT); #endif } +EXPORT_SYMBOL_GPL(sgx_kmap_epc_page); -void sgx_put_epc_page(void *epc_page_vaddr) +void sgx_kunmap_epc_page(void *epc_page_vaddr) { #ifdef CONFIG_X86_32 kunmap_atomic(epc_page_vaddr); #else #endif } +EXPORT_SYMBOL_GPL(sgx_kunmap_epc_page); struct page *sgx_get_backing(struct sgx_encl *encl, struct sgx_encl_page *entry, @@ -313,10 +315,10 @@ static int sgx_eldu(struct sgx_encl *encl, } if (!is_secs) - secs_ptr = sgx_get_epc_page(encl->secs_page.epc_page); + secs_ptr = sgx_kmap_epc_page(encl->secs_page.epc_page); - epc_ptr = sgx_get_epc_page(epc_page); - va_ptr = sgx_get_epc_page(encl_page->va_page->epc_page); + epc_ptr = sgx_kmap_epc_page(epc_page); + va_ptr = sgx_kmap_epc_page(encl_page->va_page->epc_page); pginfo.srcpge = (unsigned long)kmap_atomic(backing); pginfo.pcmd = (unsigned long)kmap_atomic(pcmd) + pcmd_offset; pginfo.linaddr = is_secs ? 0 : encl_page->addr; @@ -333,11 +335,11 @@ static int sgx_eldu(struct sgx_encl *encl, kunmap_atomic((void *)(unsigned long)(pginfo.pcmd - pcmd_offset)); kunmap_atomic((void *)(unsigned long)pginfo.srcpge); - sgx_put_epc_page(va_ptr); - sgx_put_epc_page(epc_ptr); + sgx_kunmap_epc_page(va_ptr); + sgx_kunmap_epc_page(epc_ptr); if (!is_secs) - sgx_put_epc_page(secs_ptr); + sgx_kunmap_epc_page(secs_ptr); sgx_put_backing(pcmd, false); diff --git a/drivers/platform/x86/intel_sgx_vma.c b/drivers/platform/x86/intel_sgx_vma.c index 854bd7a..d80b22a 100644 --- a/drivers/platform/x86/intel_sgx_vma.c +++ b/drivers/platform/x86/intel_sgx_vma.c @@ -132,10 +132,10 @@ static inline int sgx_vma_access_word(struct sgx_encl *encl, return -ECANCELED; if (align || (cnt != sizeof(unsigned long))) { - vaddr = sgx_get_epc_page(encl_page->epc_page); + vaddr = sgx_kmap_epc_page(encl_page->epc_page); ret = __edbgrd((void *)((unsigned long)vaddr + offset), (unsigned long *)data); - sgx_put_epc_page(vaddr); + sgx_kunmap_epc_page(vaddr); if (ret) { sgx_dbg(encl, "EDBGRD returned %d\n", ret); return -EFAULT; @@ -143,10 +143,10 @@ static inline int sgx_vma_access_word(struct sgx_encl *encl, } memcpy(data + align, buf + i, cnt); - vaddr = sgx_get_epc_page(encl_page->epc_page); + vaddr = sgx_kmap_epc_page(encl_page->epc_page); ret = __edbgwr((void *)((unsigned long)vaddr + offset), (unsigned long *)data); - sgx_put_epc_page(vaddr); + sgx_kunmap_epc_page(vaddr); if (ret) { sgx_dbg(encl, "EDBGWR returned %d\n", ret); return -EFAULT; @@ -156,10 +156,10 @@ static inline int sgx_vma_access_word(struct sgx_encl *encl, (offset + (len - i)) > 72) return -ECANCELED; - vaddr = sgx_get_epc_page(encl_page->epc_page); + vaddr = sgx_kmap_epc_page(encl_page->epc_page); ret = __edbgrd((void *)((unsigned long)vaddr + offset), (unsigned long *)data); - sgx_put_epc_page(vaddr); + sgx_kunmap_epc_page(vaddr); if (ret) { sgx_dbg(encl, "EDBGRD returned %d\n", ret); return -EFAULT;
Rename sgx_get{put}_epc_page to sgx_kmap{kunmap}_epc_page, and expose them. For normal memory, get{put}_page increases{decreases} reference count of 'struct page', and they have nothing to do with map/unmap page to kernel virtual address. Rename the equivalent for EPC to follow this convention. Signed-off-by: Kai Huang <kai.huang@linux.intel.com> --- arch/x86/include/asm/sgx.h | 2 ++ drivers/platform/x86/intel_sgx.h | 2 -- drivers/platform/x86/intel_sgx_ioctl.c | 28 ++++++++++++++-------------- drivers/platform/x86/intel_sgx_page_cache.c | 20 ++++++++++---------- drivers/platform/x86/intel_sgx_util.c | 18 ++++++++++-------- drivers/platform/x86/intel_sgx_vma.c | 12 ++++++------ 6 files changed, 42 insertions(+), 40 deletions(-)