[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Y0BEV+Xgkrln8xoh@iweiny-desk3>
Date: Fri, 7 Oct 2022 08:23:03 -0700
From: Ira Weiny <ira.weiny@...el.com>
To: Kristen Carlson Accardi <kristen@...ux.intel.com>
CC: <linux-kernel@...r.kernel.org>, <linux-sgx@...r.kernel.org>,
"Jarkko Sakkinen" <jarkko@...nel.org>,
Dave Hansen <dave.hansen@...ux.intel.com>,
Thomas Gleixner <tglx@...utronix.de>,
Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
<x86@...nel.org>, "H. Peter Anvin" <hpa@...or.com>
Subject: Re: [PATCH] x86/sgx: Replace kmap/kunmap_atomic calls
On Thu, Sep 29, 2022 at 09:06:46AM -0700, Kristen Carlson Accardi wrote:
> It is not necessary to disable page faults or preemption when
> using kmap calls, so replace kmap_atomic() and kunmap_atomic()
> calls with more the more appropriate kmap_local_page() and
> kunmap_local() calls.
>
> Signed-off-by: Kristen Carlson Accardi <kristen@...ux.intel.com>
Minor comment below.
> ---
> arch/x86/kernel/cpu/sgx/encl.c | 12 ++++++------
> arch/x86/kernel/cpu/sgx/ioctl.c | 4 ++--
> arch/x86/kernel/cpu/sgx/main.c | 8 ++++----
> 3 files changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/arch/x86/kernel/cpu/sgx/encl.c b/arch/x86/kernel/cpu/sgx/encl.c
> index f40d64206ded..63dd92bd3288 100644
> --- a/arch/x86/kernel/cpu/sgx/encl.c
> +++ b/arch/x86/kernel/cpu/sgx/encl.c
> @@ -160,8 +160,8 @@ static int __sgx_encl_eldu(struct sgx_encl_page *encl_page,
> return ret;
>
> pginfo.addr = encl_page->desc & PAGE_MASK;
> - pginfo.contents = (unsigned long)kmap_atomic(b.contents);
> - pcmd_page = kmap_atomic(b.pcmd);
> + pginfo.contents = (unsigned long)kmap_local_page(b.contents);
> + pcmd_page = kmap_local_page(b.pcmd);
> pginfo.metadata = (unsigned long)pcmd_page + b.pcmd_offset;
>
> if (secs_page)
> @@ -187,8 +187,8 @@ static int __sgx_encl_eldu(struct sgx_encl_page *encl_page,
> */
> pcmd_page_empty = !memchr_inv(pcmd_page, 0, PAGE_SIZE);
>
> - kunmap_atomic(pcmd_page);
> - kunmap_atomic((void *)(unsigned long)pginfo.contents);
> + kunmap_local(pcmd_page);
> + kunmap_local((void *)(unsigned long)pginfo.contents);
>
> get_page(b.pcmd);
> sgx_encl_put_backing(&b);
> @@ -197,10 +197,10 @@ static int __sgx_encl_eldu(struct sgx_encl_page *encl_page,
>
> if (pcmd_page_empty && !reclaimer_writing_to_pcmd(encl, pcmd_first_page)) {
> sgx_encl_truncate_backing_page(encl, PFN_DOWN(page_pcmd_off));
> - pcmd_page = kmap_atomic(b.pcmd);
> + pcmd_page = kmap_local_page(b.pcmd);
> if (memchr_inv(pcmd_page, 0, PAGE_SIZE))
> pr_warn("PCMD page not empty after truncate.\n");
> - kunmap_atomic(pcmd_page);
> + kunmap_local(pcmd_page);
> }
>
> put_page(b.pcmd);
> diff --git a/arch/x86/kernel/cpu/sgx/ioctl.c b/arch/x86/kernel/cpu/sgx/ioctl.c
> index ebe79d60619f..f2f918b8b9b1 100644
> --- a/arch/x86/kernel/cpu/sgx/ioctl.c
> +++ b/arch/x86/kernel/cpu/sgx/ioctl.c
> @@ -221,11 +221,11 @@ static int __sgx_encl_add_page(struct sgx_encl *encl,
> pginfo.secs = (unsigned long)sgx_get_epc_virt_addr(encl->secs.epc_page);
> pginfo.addr = encl_page->desc & PAGE_MASK;
> pginfo.metadata = (unsigned long)secinfo;
> - pginfo.contents = (unsigned long)kmap_atomic(src_page);
> + pginfo.contents = (unsigned long)kmap_local_page(src_page);
>
> ret = __eadd(&pginfo, sgx_get_epc_virt_addr(epc_page));
>
> - kunmap_atomic((void *)pginfo.contents);
> + kunmap_local((void *)pginfo.contents);
> put_page(src_page);
>
> return ret ? -EIO : 0;
> diff --git a/arch/x86/kernel/cpu/sgx/main.c b/arch/x86/kernel/cpu/sgx/main.c
> index 515e2a5f25bb..4efda5e8cadf 100644
> --- a/arch/x86/kernel/cpu/sgx/main.c
> +++ b/arch/x86/kernel/cpu/sgx/main.c
> @@ -159,17 +159,17 @@ static int __sgx_encl_ewb(struct sgx_epc_page *epc_page, void *va_slot,
> pginfo.addr = 0;
> pginfo.secs = 0;
>
> - pginfo.contents = (unsigned long)kmap_atomic(backing->contents);
> - pginfo.metadata = (unsigned long)kmap_atomic(backing->pcmd) +
> + pginfo.contents = (unsigned long)kmap_local_page(backing->contents);
> + pginfo.metadata = (unsigned long)kmap_local_page(backing->pcmd) +
> backing->pcmd_offset;
>
> ret = __ewb(&pginfo, sgx_get_epc_virt_addr(epc_page), va_slot);
> set_page_dirty(backing->pcmd);
> set_page_dirty(backing->contents);
>
> - kunmap_atomic((void *)(unsigned long)(pginfo.metadata -
> + kunmap_local((void *)(unsigned long)(pginfo.metadata -
> backing->pcmd_offset));
For kunmap_local() one can use any address in the page. So this can be:
kunmap_local((void *)(unsigned long)(pginfo.metadata));
Regardless:
Reviewed-by: Ira Weiny <ira.weiny@...el.com>
> - kunmap_atomic((void *)(unsigned long)pginfo.contents);
> + kunmap_local((void *)(unsigned long)pginfo.contents);
>
> return ret;
> }
> --
> 2.37.3
>
Powered by blists - more mailing lists