linux-kernel - Re: [PATCH] x86/sgx: Replace kmap/kunmap

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Y0biN3/JsZMa0yUr@kernel.org>
Date:   Wed, 12 Oct 2022 18:50:15 +0300
From:   Jarkko Sakkinen <jarkko@...nel.org>
To:     Dave Hansen <dave.hansen@...el.com>
Cc:     Ira Weiny <ira.weiny@...el.com>,
        Kristen Carlson Accardi <kristen@...ux.intel.com>,
        linux-kernel@...r.kernel.org, linux-sgx@...r.kernel.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 Wed, Oct 12, 2022 at 05:50:19PM +0300, Jarkko Sakkinen wrote:
> On Wed, Oct 12, 2022 at 07:13:26AM -0700, Dave Hansen wrote:
> > On 10/12/22 00:15, Jarkko Sakkinen wrote:
> > > There's no data to show that this change would be useful to do.
> > 
> > Jarkko, I think the overall transition to kmap_local_page() is a good
> > one.  It is a superior API and having it around will pave the way for
> > new features.  I don't think we should demand 'data' for each and every
> > one of these.
> > 
> > Please take a look around the tree and see how other maintainers are
> > handling these patches.  They're not limited to SGX.
> 
> Sure, I'll take a look for comparison.

Yeah, I think it is pretty solid idea.

Looking at the decription:

"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."

We did not pick kmap_atomic() because it disables preeemption,
i.e. it was not a "design choice". I'd rather phrase this as
along the lines:

"Migrate to the newer kmap_local_page() interface from kmap_atomic()
in order to move away from per-CPU maps to pre-task_struct maps.
This in effect removes the need to disable preemption in the
local CPU while kmap is active, and thus vastly reduces overall
system latency."

Can be improved or written completely otherwise. I just wrote it
in the way that I had understood the whole deal in the first place.

BR, Jarkko