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] [day] [month] [year] [list]

Message-ID: <26cfc85cf1beebc600428e24f12f49c961540071.camel@linux.intel.com>
Date:   Thu, 13 Oct 2022 09:03:01 -0700
From:   Kristen Carlson Accardi <kristen@...ux.intel.com>
To:     Jarkko Sakkinen <jarkko@...nel.org>,
        Dave Hansen <dave.hansen@...el.com>
Cc:     Ira Weiny <ira.weiny@...el.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, 2022-10-12 at 18:50 +0300, Jarkko Sakkinen wrote:
> 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

Thanks for looking into this Jarkko - I will update the commit log for
the next version.

Kristen