linux-kernel - Re: [patch V3 02/11] x86/mm/cpa: Split, rename and clean up try_preserve_large

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: <alpine.DEB.2.21.1809190034060.1468@nanos.tec.linutronix.de>
Date:   Wed, 19 Sep 2018 00:34:54 +0200 (CEST)
From:   Thomas Gleixner <tglx@...utronix.de>
To:     Peter Zijlstra <peterz@...radead.org>
cc:     LKML <linux-kernel@...r.kernel.org>, x86@...nel.org,
        Bin Yang <bin.yang@...el.com>,
        Dave Hansen <dave.hansen@...el.com>,
        Mark Gross <mark.gross@...el.com>
Subject: Re: [patch V3 02/11] x86/mm/cpa: Split, rename and clean up
 try_preserve_large_page()

On Tue, 18 Sep 2018, Peter Zijlstra wrote:
> On Tue, Sep 18, 2018 at 10:19:09AM +0200, Peter Zijlstra wrote:
> > On Mon, Sep 17, 2018 at 04:29:08PM +0200, Thomas Gleixner wrote:
> > > @@ -1288,23 +1287,23 @@ static int __change_page_attr(struct cpa
> > >  	err = split_large_page(cpa, kpte, address);
> > >  	if (!err) {
> > >  		/*
> > > +		 * Do a global flush tlb after splitting the large page
> > > +		 * and before we do the actual change page attribute in the PTE.
> > > +		 *
> > > +		 * With out this, we violate the TLB application note, that says
> > > +		 * "The TLBs may contain both ordinary and large-page
> > >  		 *  translations for a 4-KByte range of linear addresses. This
> > >  		 *  may occur if software modifies the paging structures so that
> > >  		 *  the page size used for the address range changes. If the two
> > >  		 *  translations differ with respect to page frame or attributes
> > >  		 *  (e.g., permissions), processor behavior is undefined and may
> > >  		 *  be implementation-specific."
> > > +		 *
> > > +		 * We do this global tlb flush inside the cpa_lock, so that we
> > >  		 * don't allow any other cpu, with stale tlb entries change the
> > >  		 * page attribute in parallel, that also falls into the
> > >  		 * just split large page entry.
> > > +		 */
> > >  		flush_tlb_all();
> > >  		goto repeat;
> > >  	}
> > 
> > this made me look at the tlb invalidation of that thing again; do we
> > want something like the below?
> > 
>
> Further cleanups are possible...

Yes please. Can you write up a changelog for that please?

Thanks,

	tglx