Date: Mon, 8 Jan 2018 19:12:40 +0100
From: Willy Tarreau <w@1wt.eu>
To: Dave Hansen <dave.hansen@intel.com>
Cc: linux-kernel@vger.kernel.org, x86@kernel.org, tglx@linutronix.de,
        gnomes@lxorguk.ukuu.org.uk, torvalds@linux-foundation.org,
        Andy Lutomirski <luto@kernel.org>
Subject: Re: [PATCH RFC 4/4] x86/entry/pti: don't switch PGD on tasks holding
 flag TIF_NOPTI
Message-ID: <20180108181240.GK10913@1wt.eu>
References: <1515427939-10999-1-git-send-email-w@1wt.eu>
 <1515427939-10999-5-git-send-email-w@1wt.eu>
 <9d6917dc-1a6c-a8d4-95fe-f767495f99d3@intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <9d6917dc-1a6c-a8d4-95fe-f767495f99d3@intel.com>
User-Agent: Mutt/1.6.1 (2016-04-27)
Sender: linux-kernel-owner@vger.kernel.org

On Mon, Jan 08, 2018 at 09:20:08AM -0800, Dave Hansen wrote:
> > @@ -214,6 +215,11 @@
> >  .macro SWITCH_TO_KERNEL_CR3 scratch_reg:req
> >  	ALTERNATIVE "jmp .Lend_\@", "", X86_FEATURE_PTI
> >  	mov	%cr3, \scratch_reg
> > +
> > +	/* if we're already on the kernel PGD, we don't switch */
> > +	testq $(PTI_SWITCH_PGTABLES_MASK), \scratch_reg
> > +	jz .Lend_\@
> > +
> >  	ADJUST_KERNEL_CR3 \scratch_reg
> >  	mov	\scratch_reg, %cr3
> >  .Lend_\@:
> 
> This is an optimization that we can do generally without your feature.
> Actually, it would be a welcome bit of benchmarking if you could see if
> just this hunk helps your workload.

Well, this alone provides no gains at all, as I can see when running
haproxy without the nopti wrapper. And it makes sense, as there is no
reason without this patch series for coming from userspace with CR3
pointing to the kernel's PGD.

> You touched on it in the description, but this is a *very* clever way to
> do what you need without needing to look at the task flag at
> user->kernel entry which also happens to be a place you don't have
> task_struct mapped.  It *greatly* simplifies what this would have to do
> otherwise.

In fact when I saw my kernel fail to boot when trying to read the task
flag, I quickly realized I needed something else to check before
entering the kernel ;-)

Willy