[intel-sgx-kernel-dev,4/7] intel_sgx: expose sgx_kmap{kunmap}_epc_page
diff mbox

Message ID 20170404072938.4800-5-kai.huang@linux.intel.com
State New
Headers show

Commit Message

Kai Huang April 4, 2017, 7:29 a.m. UTC
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(-)

Comments

Jarkko Sakkinen April 4, 2017, 2:51 p.m. UTC | #1
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
>
Kai Huang April 5, 2017, 12:16 a.m. UTC | #2
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
>
Jarkko Sakkinen April 5, 2017, 7:38 a.m. UTC | #3
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
Kai Huang April 6, 2017, 9 a.m. UTC | #4
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
>
Jarkko Sakkinen April 6, 2017, 6:46 p.m. UTC | #5
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
Kai Huang April 7, 2017, 5:38 a.m. UTC | #6
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
>

Patch
diff mbox

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;