Re: [PATCH 0/8] use struct pt_regs based syscall calling for x86-64

From: Dominik Brodowski <linux@dominikbrodowski.net>
To: Ingo Molnar <mingo@kernel.org>
Cc: linux-kernel@vger.kernel.org, Al Viro <viro@zeniv.linux.org.uk>,
	Andi Kleen <ak@linux.intel.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Andy Lutomirski <luto@kernel.org>,
	Brian Gerst <brgerst@gmail.com>,
	Denys Vlasenko <dvlasenk@redhat.com>,
	"H. Peter Anvin" <hpa@zytor.com>, Ingo Molnar <mingo@redhat.com>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Peter Zijlstra <peterz@infradead.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	x86@kernel.org
Subject: Re: [PATCH 0/8] use struct pt_regs based syscall calling for x86-64
Date: Fri, 6 Apr 2018 10:34:36 +0200	[thread overview]
Message-ID: <20180406083436.GA17948@isilmar-4.linta.de> (raw)
In-Reply-To: <20180406082322.ud37sj3b5ldm7xxd@gmail.com>

On Fri, Apr 06, 2018 at 10:23:22AM +0200, Ingo Molnar wrote:
> 
> * Dominik Brodowski <linux@dominikbrodowski.net> wrote:
> 
> > On Thu, Apr 05, 2018 at 05:19:33PM +0200, Ingo Molnar wrote:
> > > Ok, this series looks mostly good to me, but AFAICS this breaks the UML build:
> > > 
> > >  make[2]: *** No rule to make target 'archheaders'.  Stop.
> > >  arch/um/Makefile:119: recipe for target 'archheaders' failed
> > >  make[1]: *** [archheaders] Error 2
> > >  make[1]: *** Waiting for unfinished jobs....
> > 
> > Ah, that's caused by patch 8/8 which I did and do not like all that much
> > anyway: UML re-uses syscall_64.tbl which now has x86-specific entries like
> > __sys_x86_pread64, but expects the generic syscall stub sys_pread64
> > referenced there. Fixup patch below; could be folded with patch 8/8. Or
> > patch 8/8 could simply be dropped from the series altogether...
> 
> I still like the 'truth in advertising' aspect. For example if I see this in the 
> syscall table:
> 
>  10      common  mprotect                __sys_x86_mprotect
> 
> I can immediately find the _real_ syscall entry point:
> 
> ffffffff81180a10 <__sys_x86_mprotect>:
> ffffffff81180a10:       48 8b 57 60             mov    0x60(%rdi),%rdx
> ffffffff81180a14:       48 8b 77 68             mov    0x68(%rdi),%rsi
> ffffffff81180a18:       b9 ff ff ff ff          mov    $0xffffffff,%ecx
> ffffffff81180a1d:       48 8b 7f 70             mov    0x70(%rdi),%rdi
> ffffffff81180a21:       e8 fa fc ff ff          callq  ffffffff81180720 <do_mprotect_pkey>
> ffffffff81180a26:       48 98                   cltq   
> ffffffff81180a28:       c3                      retq   
> ffffffff81180a29:       0f 1f 80 00 00 00 00    nopl   0x0(%rax)
> 
> If, on the other hand, I see this entry:
> 
>  10     common  mprotect                sys_mprotect
> 
> Then, as a first step, no symbol anywhere matches with this:
> 
>  triton:~/tip> grep sys_mprotect System.map 
>  triton:~/tip> 
> 
> "sys_mprotect" does not exist in any easily discoverable sense. You have to *know* 
> to replace the sys_ prefix with __sys_x86_ to find it.
> 
> Now arguably we could use a __sys_ prefix instead of the grep-barrier __sys_x86 
> prefix - but that too would be somewhat confusing I think.

Well, if looking at the ARCH="um" kernel, you won't find the
__sys_x86_mprotect there in its System.map -- so we either have to
disentangle um and plain x86, or live with some cause for confusion.

__sys_mprotect as prefix won't work by the way, as the double-underscore
__sys_ variant is already used in net/* for internal syscall helpers.

Thanks,
	Dominik