From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754282AbdK1Uem (ORCPT ); Tue, 28 Nov 2017 15:34:42 -0500 Received: from mail-it0-f47.google.com ([209.85.214.47]:41837 "EHLO mail-it0-f47.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752534AbdK1Uej (ORCPT ); Tue, 28 Nov 2017 15:34:39 -0500 X-Google-Smtp-Source: AGs4zMZo6TCnwudRb/IznSiZUCwVbWgXilQ3Xy2dxUykOKQJYzkOFSnf0DOLVUOIZ4gee1/otdk/RTtvBG8hfGCTnc0= MIME-Version: 1.0 In-Reply-To: <20171128190505.pqip2v2ewb3isjhd@hirez.programming.kicks-ass.net> References: <20171127104923.14378-1-mingo@kernel.org> <20171127104923.14378-16-mingo@kernel.org> <20171128163908.e3gj6zgq6kcbdlxe@hirez.programming.kicks-ass.net> <1c9a32ed-e754-9d91-98f7-b72c07b2c0dc@linux.intel.com> <20171128190505.pqip2v2ewb3isjhd@hirez.programming.kicks-ass.net> From: Andy Lutomirski Date: Tue, 28 Nov 2017 12:34:17 -0800 Message-ID: Subject: Re: [PATCH 15/24] x86/mm: Allow flushing for future ASID switches To: Peter Zijlstra Cc: Dave Hansen , Ingo Molnar , "linux-kernel@vger.kernel.org" , Thomas Gleixner , "H . Peter Anvin" , Borislav Petkov , Linus Torvalds Content-Type: text/plain; charset="UTF-8" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Nov 28, 2017 at 11:05 AM, Peter Zijlstra wrote: > On Tue, Nov 28, 2017 at 10:13:30AM -0800, Dave Hansen wrote: >> Thanks for looking at this, Peter. I've been resisting doing this for a >> bit and it's an embarrassingly small amount of code. > > Right, well, its not complete yet, and it might be complete crap :-) > >> On 11/28/2017 08:39 AM, Peter Zijlstra wrote: >> > @@ -220,7 +221,21 @@ For 32-bit we have the following conventions - kernel is built with >> > .macro SWITCH_TO_USER_CR3 scratch_reg:req >> > STATIC_JUMP_IF_FALSE .Lend_\@, kaiser_enabled_key, def=1 >> > mov %cr3, \scratch_reg >> > - ADJUST_USER_CR3 \scratch_reg >> > + push \scratch_reg >> >> Do we have a good stack in all the spots that we need to do this? It >> may have changed with the trampoline stack, but I'm 100% sure that it >> wasn't so in the recent past. > > Dunno really. I figured I'd give it a go and see what happens. So far > the machine still works. But I was hoping Andy would have an opinion on > this. I thought we had a stack in all these places even before the trampoline. There was an issue with *entry*, but I think exit has always been okay. > >> Let me see if I'm reading the assembly right. > > Yep, seems you can read asm :-) > > >> > +DECLARE_PER_CPU(unsigned long, __asid_flush); >> >> Could we spare enough space to make this something like >> user_asid_flush_pending_mask? > > Yeah, if I can get it all working we'll bikeshed on a name ;-) > >> It took me a minute to realize that it was a mask. Also, since we only >> have 6 asids, should we bit a bit more stingy with the type? > > I picked unsigned long because our bitops (__set_bit in this case, use > it), and I know we're LE and could simply use a shorter type, but meh. > >> It took me a minute to realize that mixing these is still OK, even if >> the mm associated with the ASID changes. It's because once the ASID is >> stale, it doesn't matter *why* it is stale. Just that the next guy who >> *uses* it needs to do the flush. You can do 1,000 tlb flushes, a >> context switch, a tlb flush and another context switch, but if you only >> go out to userspace once, you only need 1 ASID flush. That fits >> perfectly with this bit that gets set a bunch of times and only cleared >> once at exit to userspace. > > Just so. > > I'm now staring at the RESTORE_CR3 stuff, and that appears to be called > in the NMI handling where the stack is not to be used (if I read it > right), so that's going to be a little more tricky. I think it should be fine. A very old version of the patches had that problem, but, in -tip, the nmi RESTORE_CR3 is in the fancy recursion-protected region, and the stack is okay. The idea is that we're already on the old (possibly user) CR3 before we do the crazy recursion-checking bits. But that's fine, since all that's accessed there is the IST stack, and that's in the cpu_entry_area and thus safe regardless of CR3. Side question: on extremely quick read, you're doing bt then btr. Why not just do a single btr and be done with it? Are you trying to avoid getting exclusive access to the cacheline when not needed?