From: Ingo Molnar <mingo@kernel.org>
To: Dominik Brodowski <linux@dominikbrodowski.net>
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 11:20:46 +0200 [thread overview]
Message-ID: <20180406092046.2jqjdzvecmfm6sif@gmail.com> (raw)
In-Reply-To: <20180406083436.GA17948@isilmar-4.linta.de>
* Dominik Brodowski <linux@dominikbrodowski.net> wrote:
> 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.
I'm primarily concerned about everything making sense on x86 - UML is an entirely
separate architecture with heavy tradeoffs and kludges.
> __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.
Ok - then triple underscore - but overall I think it's more confusing.
Btw., what was the problem with calling the x86 ptregs wrapper sys_mprotect?
The only reason I suggested the __sys_x86_ prefix was because you originally
suggested that there's symbol name overlap, but I don't think that's the case
within the same kernel build, as the regular non-ptregs prototype:
asmlinkage long sys_mprotect(unsigned long start, size_t len, unsigned long prot);
... will only exist on !CONFIG_ARCH_HAS_SYSCALL_WRAPPER kernels.
So maybe that's the simplest and least confusing solution.
Thanks,
Ingo
next prev parent reply other threads:[~2018-04-06 9:20 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-04-05 9:52 [PATCH 0/8] use struct pt_regs based syscall calling for x86-64 Dominik Brodowski
2018-04-05 9:53 ` [PATCH 1/8] x86: don't pointlessly reload the system call number Dominik Brodowski
2018-04-06 17:09 ` [tip:x86/asm] x86/syscalls: Don't " tip-bot for Linus Torvalds
2018-04-05 9:53 ` [PATCH 2/8] syscalls: introduce CONFIG_ARCH_HAS_SYSCALL_WRAPPER Dominik Brodowski
2018-04-06 17:10 ` [tip:x86/asm] syscalls/core: Introduce CONFIG_ARCH_HAS_SYSCALL_WRAPPER=y tip-bot for Dominik Brodowski
2018-04-05 9:53 ` [PATCH 3/8] syscalls/x86: use struct pt_regs based syscall calling for 64-bit syscalls Dominik Brodowski
2018-04-06 17:11 ` [tip:x86/asm] syscalls/x86: Use 'struct pt_regs' based syscall calling convention " tip-bot for Dominik Brodowski
2018-04-05 9:53 ` [PATCH 4/8] syscalls: prepare ARCH_HAS_SYSCALL_WRAPPER for compat syscalls Dominik Brodowski
2018-04-06 17:11 ` [tip:x86/asm] syscalls/core: Prepare CONFIG_ARCH_HAS_SYSCALL_WRAPPER=y " tip-bot for Dominik Brodowski
2018-04-05 9:53 ` [PATCH 5/8] syscalls/x86: use struct pt_regs based syscall calling for IA32_EMULATION and x32 Dominik Brodowski
2018-04-06 17:12 ` [tip:x86/asm] syscalls/x86: Use 'struct pt_regs' " tip-bot for Dominik Brodowski
2018-04-05 9:53 ` [PATCH 6/8] syscalls/x86: unconditionally enable struct pt_regs based syscalls on x86_64 Dominik Brodowski
2018-04-06 17:12 ` [tip:x86/asm] syscalls/x86: Unconditionally enable 'struct pt_regs' " tip-bot for Dominik Brodowski
2018-04-05 9:53 ` [PATCH 7/8] x86/entry/64: extend register clearing on syscall entry to lower registers Dominik Brodowski
2018-04-06 17:13 ` [tip:x86/asm] syscalls/x86: Extend " tip-bot for Dominik Brodowski
2018-04-05 9:53 ` [PATCH 8/8] syscalls/x86: rename struct pt_regs-based sys_*() to __sys_x86_*() Dominik Brodowski
2018-04-05 18:35 ` kbuild test robot
2018-04-05 15:19 ` [PATCH 0/8] use struct pt_regs based syscall calling for x86-64 Ingo Molnar
2018-04-05 20:31 ` Dominik Brodowski
2018-04-06 8:23 ` Ingo Molnar
2018-04-06 8:31 ` Dominik Brodowski
2018-04-06 8:34 ` Dominik Brodowski
2018-04-06 9:20 ` Ingo Molnar [this message]
2018-04-06 9:34 ` Dominik Brodowski
2018-04-06 12:34 ` Ingo Molnar
2018-04-06 13:07 ` Dominik Brodowski
2018-04-06 17:03 ` Ingo Molnar
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20180406092046.2jqjdzvecmfm6sif@gmail.com \
--to=mingo@kernel.org \
--cc=ak@linux.intel.com \
--cc=akpm@linux-foundation.org \
--cc=brgerst@gmail.com \
--cc=dvlasenk@redhat.com \
--cc=hpa@zytor.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux@dominikbrodowski.net \
--cc=luto@kernel.org \
--cc=mingo@redhat.com \
--cc=peterz@infradead.org \
--cc=tglx@linutronix.de \
--cc=torvalds@linux-foundation.org \
--cc=viro@zeniv.linux.org.uk \
--cc=x86@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.