From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754786AbbCMWbr (ORCPT ); Fri, 13 Mar 2015 18:31:47 -0400 Received: from relay4-d.mail.gandi.net ([217.70.183.196]:40657 "EHLO relay4-d.mail.gandi.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752033AbbCMWbq (ORCPT ); Fri, 13 Mar 2015 18:31:46 -0400 Date: Fri, 13 Mar 2015 15:31:39 -0700 From: josh@joshtriplett.org To: Andy Lutomirski Cc: Al Viro , Andrew Morton , Ingo Molnar , Kees Cook , Oleg Nesterov , "Paul E. McKenney" , "H. Peter Anvin" , Rik van Riel , Thomas Gleixner , Thiago Macieira , Michael Kerrisk , "linux-kernel@vger.kernel.org" , Linux API , Linux FS Devel , X86 ML Subject: Re: [PATCH 2/6] x86: Opt into HAVE_COPY_THREAD_TLS, for both 32-bit and 64-bit Message-ID: <20150313223139.GD10954@cloud> References: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.20 (2009-06-14) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Fri, Mar 13, 2015 at 03:01:16PM -0700, Andy Lutomirski wrote: > On Thu, Mar 12, 2015 at 6:40 PM, Josh Triplett wrote: > > For 32-bit userspace on a 64-bit kernel, this requires modifying > > stub32_clone to actually swap the appropriate arguments to match > > CONFIG_CLONE_BACKWARDS, rather than just leaving the C argument for tls > > broken. > > > > Signed-off-by: Josh Triplett > > Signed-off-by: Thiago Macieira > > --- > > arch/x86/Kconfig | 1 + > > arch/x86/ia32/ia32entry.S | 2 +- > > arch/x86/kernel/process_32.c | 6 +++--- > > arch/x86/kernel/process_64.c | 8 ++++---- > > 4 files changed, 9 insertions(+), 8 deletions(-) > > > > diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig > > index b7d31ca..4960b0d 100644 > > --- a/arch/x86/Kconfig > > +++ b/arch/x86/Kconfig > > @@ -124,6 +124,7 @@ config X86 > > select MODULES_USE_ELF_REL if X86_32 > > select MODULES_USE_ELF_RELA if X86_64 > > select CLONE_BACKWARDS if X86_32 > > + select HAVE_COPY_THREAD_TLS > > select ARCH_USE_BUILTIN_BSWAP > > select ARCH_USE_QUEUE_RWLOCK > > select OLD_SIGSUSPEND3 if X86_32 || IA32_EMULATION > > diff --git a/arch/x86/ia32/ia32entry.S b/arch/x86/ia32/ia32entry.S > > index 156ebca..0286735 100644 > > --- a/arch/x86/ia32/ia32entry.S > > +++ b/arch/x86/ia32/ia32entry.S > > @@ -487,7 +487,7 @@ GLOBAL(\label) > > ALIGN > > GLOBAL(stub32_clone) > > leaq sys_clone(%rip),%rax > > - mov %r8, %rcx > > + xchg %r8, %rcx > > jmp ia32_ptregs_common > > Do I understand correct that whatever function this is a stub for just > takes its arguments in the wrong order? If so, can we just fix it > instead of using xchg here? 32-bit x86 and 64-bit x86 take the arguments to clone in a different order, and stub32_clone fixes up the argument order then calls the 64-bit sys_clone. I'd love to see *all* the 32-on-64 compat stubs for clone rewritten in C under CONFIG_COMPAT. However, doing so would require encoding the knowledge for each 64-bit architecture for how its corresponding 32-bit architecture accepts arguments to clone, which is information that the current CONFIG_CLONE_BACKWARDS{1,2,3} don't include; it would then require cleaning up all the architecture-specific assembly stubs for 32-bit clone entry points. In the meantime, doing that *just* for 32-bit x86 on 64-bit x86 doesn't seem worth it, since it would require adding a new C entry point for compat_sys_clone under arch/x86 somewhere. One cleanup at a time. :) > In general, I much prefer C code to new asm where it makes sense to > make this tradeoff. Agreed completely. However, this is at least conservation-of-asm, or reduction if you consider the pt_regs argument-grabbing hack to be asm-esque code. > Other than that, this is a huge improvement. You'll have minor > conflicts against -tip, though. Right, I've seen your current changes there. Should be a trivial merge though. Would you mind providing an ack for the series, or at least for the first two patches? (I'm wondering whose tree this series ought to go through, for that matter.) - Josh Triplett