Re: [PATCH v3 16/17] x86/sgx: Introduce sgx_encl_get_backing()

[Sean Christopherson <sean.j.christopherson@intel.com>]

On Mon, Sep 16, 2019 at 01:18:02PM +0300, Jarkko Sakkinen wrote:
> Since __sgx_encl_eldu() and __sgx_encl_ewb() pin the backing storage in the
> same way, consolidate this to sgx_encl_get_backing() replacing
> sgx_encl_get_backing_page().
> 
> Cc: Sean Christopherson <sean.j.christopherson@intel.com>
> Cc: Shay Katz-zamir <shay.katz-zamir@intel.com>
> Cc: Serge Ayoun <serge.ayoun@intel.com>
> Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
> ---
>  arch/x86/kernel/cpu/sgx/encl.c    | 86 +++++++++++++++++++------------
>  arch/x86/kernel/cpu/sgx/encl.h    | 20 +++----
>  arch/x86/kernel/cpu/sgx/reclaim.c | 39 ++++----------
>  3 files changed, 71 insertions(+), 74 deletions(-)
> 
> diff --git a/arch/x86/kernel/cpu/sgx/encl.c b/arch/x86/kernel/cpu/sgx/encl.c
> index 66762b9c1517..a71414ce05b1 100644
> --- a/arch/x86/kernel/cpu/sgx/encl.c
> +++ b/arch/x86/kernel/cpu/sgx/encl.c
> @@ -17,11 +17,8 @@ static int __sgx_encl_eldu(struct sgx_encl_page *encl_page,
>  	unsigned long va_offset = SGX_ENCL_PAGE_VA_OFFSET(encl_page);
>  	struct sgx_encl *encl = encl_page->encl;
>  	struct sgx_pageinfo pginfo;
> -	unsigned long pcmd_offset;
> -	struct page *backing;
> +	struct sgx_backing b;
>  	pgoff_t page_index;
> -	pgoff_t pcmd_index;
> -	struct page *pcmd;
>  	int ret;
>  
>  	if (secs_page)
> @@ -29,24 +26,14 @@ static int __sgx_encl_eldu(struct sgx_encl_page *encl_page,
>  	else
>  		page_index = PFN_DOWN(encl->size);
>  
> -	pcmd_index = sgx_pcmd_index(encl, page_index);
> -	pcmd_offset = sgx_pcmd_offset(page_index);
> -
> -	backing = sgx_encl_get_backing_page(encl, page_index);
> -	if (IS_ERR(backing)) {
> -		ret = PTR_ERR(backing);
> -		goto err_backing;
> -	}
> -
> -	pcmd = sgx_encl_get_backing_page(encl, pcmd_index);
> -	if (IS_ERR(pcmd)) {
> -		ret = PTR_ERR(pcmd);
> -		goto err_pcmd;
> -	}
> +	ret = sgx_encl_get_backing(encl, page_index, &b);
> +	if (ret)
> +		return ret;
>  
>  	pginfo.addr = SGX_ENCL_PAGE_ADDR(encl_page);
> -	pginfo.contents = (unsigned long)kmap_atomic(backing);
> -	pginfo.metadata = (unsigned long)kmap_atomic(pcmd) + pcmd_offset;
> +	pginfo.contents = (unsigned long)kmap_atomic(b.contents);
> +	pginfo.metadata = (unsigned long)kmap_atomic(b.pcmd) +
> +			  b.pcmd_offset;
>  
>  	if (secs_page)
>  		pginfo.secs = (u64)sgx_epc_addr(secs_page);
> @@ -62,15 +49,12 @@ static int __sgx_encl_eldu(struct sgx_encl_page *encl_page,
>  		ret = -EFAULT;
>  	}
>  
> -	kunmap_atomic((void *)(unsigned long)(pginfo.metadata - pcmd_offset));
> +	kunmap_atomic((void *)(unsigned long)(pginfo.metadata - b.pcmd_offset));
>  	kunmap_atomic((void *)(unsigned long)pginfo.contents);
>  
> -	put_page(pcmd);
> +	put_page(b.pcmd);
> +	put_page(b.contents);

If we're going to have sgx_encl_get_backing(), it should be paired with
sgx_encl_put_backing(), e.g. it's not immediately obvious whose doing the
"get page" corresponding to the put_page() calls.