From: Ingo Molnar <mingo@kernel.org>
To: Richard Weinberger <richard.weinberger@gmail.com>
Cc: "Andy Lutomirski" <luto@kernel.org>,
"x86@kernel.org" <x86@kernel.org>,
LKML <linux-kernel@vger.kernel.org>,
"Frédéric Weisbecker" <fweisbec@gmail.com>,
"Rik van Riel" <riel@redhat.com>,
"Oleg Nesterov" <oleg@redhat.com>,
"Denys Vlasenko" <vda.linux@googlemail.com>,
"Borislav Petkov" <bp@alien8.de>,
"Kees Cook" <keescook@chromium.org>,
"Brian Gerst" <brgerst@gmail.com>,
"Linus Torvalds" <torvalds@linux-foundation.org>,
"Denys Vlasenko" <dvlasenk@redhat.com>
Subject: Re: [RFC/INCOMPLETE 00/13] x86: Rewrite exit-to-userspace code
Date: Wed, 17 Jun 2015 13:04:50 +0200 [thread overview]
Message-ID: <20150617110450.GA8919@gmail.com> (raw)
In-Reply-To: <CAFLxGvx6tnmqojFHYX9A3j_9dN8E-rmR25a9br59Yf7OuniP-Q@mail.gmail.com>
* Richard Weinberger <richard.weinberger@gmail.com> wrote:
> On Wed, Jun 17, 2015 at 11:48 AM, Ingo Molnar <mingo@kernel.org> wrote:
> >
> > * Andy Lutomirski <luto@kernel.org> wrote:
> >
> >> This is incomplete, but it's finally good enough that I think it's
> >> time to get other opinions on it. It is a complete rewrite of the
> >> slow path code that handles exits to user mode.
> >
> > Modulo the small comments I made about the debug checks interface plus naming
> > details the structure and intention of this series gives me warm fuzzy feelings.
> >
> >> The exit-to-usermode code is copied in several places and is written in a nasty
> >> combination of asm and C. It's not at all clear what it's supposed to do, and
> >> the way it's structured makes it very hard to work with. For example, it's not
> >> even clear why syscall exit hooks are called only once per syscall right now.
> >> (It seems to be a side effect of the way that rdi and rdx are handled in the asm
> >> loop, and it seems reliable, but it's still pointlessly complicated.) The
> >> existing code also makes context tracking overly complicated and hard to
> >> understand. Finally, it's nearly impossible for anyone to change what happens
> >> on exit to usermode, since the existing code is so fragile.
> >
> > Amen.
> >
> >> I tried to clean it up incrementally, but I decided it was too hard. Instead,
> >> this series just replaces the code. It seems to work.
> >
> > Any known bugs beyond UML build breakage?
> >
> >> Context tracking in particular works very differently now. The low-level entry
> >> code checks that we're in CONTEXT_USER and switches to CONTEXT_KERNEL. The exit
> >> code does the reverse. There is no need to track what CONTEXT_XYZ state we came
> >> from, because we already know. Similarly, SCHEDULE_USER is gone, since we can
> >> reschedule if needed by simply calling schedule() from C code.
> >>
> >> The main things that are missing are that I haven't done the 32-bit parts
> >> (anyone want to help?) and therefore I haven't deleted the old C code. I also
> >> think this may break UML for trivial reasons.
> >>
> >> Because I haven't converted the 32-bit code yet, all of the now-unnecessary
> >> unnecessary calls to exception_enter are still present in traps.c.
> >>
> >> IRQ context tracking is still duplicated. We should probably clean it up by
> >> changing the core code to supply something like
> >> irq_enter_we_are_already_in_context_kernel.
> >>
> >> Thoughts?
> >
> > So assuming you fix the UML build I'm inclined to go for it, even in this
> > incomplete form, to increase testing coverage.
>
> Andy, can you please share the build breakage you're facing?
> I'll happily help you fixing it.
So they come in the form of:
./arch/um/include/shared/kern_util.h:25:12: error: conflicting types for ‘do_signal’
which comes from now x86 also having a do_signal().
The patch below fixes it by harmonizing the UML implementation with the x86 one.
This improves the UML side a bit, and fixes the build failure.
Thanks,
Ingo
=========================>
Subject: uml: Fix do_signal() prototype
From: Ingo Molnar <mingo@kernel.org>
Date: Wed Jun 17 12:58:37 CEST 2015
Now that x86 exports its do_signal(), the prototypes clash.
Fix the clash and also improve the code a bit: remove the unnecessary
kern_do_signal() indirection. This allows interrupt_end() to share
the 'regs' parameter calculation.
Also remove the unused return code to match x86.
Minimally build and boot tested.
Cc: Richard Weinberger <richard.weinberger@gmail.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Brian Gerst <brgerst@gmail.com>
Cc: Denys Vlasenko <dvlasenk@redhat.com>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
arch/um/include/shared/kern_util.h | 3 ++-
arch/um/kernel/process.c | 6 ++++--
arch/um/kernel/signal.c | 8 +-------
arch/um/kernel/tlb.c | 2 +-
arch/um/kernel/trap.c | 2 +-
5 files changed, 9 insertions(+), 12 deletions(-)
Index: tip/arch/um/include/shared/kern_util.h
===================================================================
--- tip.orig/arch/um/include/shared/kern_util.h
+++ tip/arch/um/include/shared/kern_util.h
@@ -22,7 +22,8 @@ extern int kmalloc_ok;
extern unsigned long alloc_stack(int order, int atomic);
extern void free_stack(unsigned long stack, int order);
-extern int do_signal(void);
+struct pt_regs;
+extern void do_signal(struct pt_regs *regs);
extern void interrupt_end(void);
extern void relay_signal(int sig, struct siginfo *si, struct uml_pt_regs *regs);
Index: tip/arch/um/kernel/process.c
===================================================================
--- tip.orig/arch/um/kernel/process.c
+++ tip/arch/um/kernel/process.c
@@ -90,12 +90,14 @@ void *__switch_to(struct task_struct *fr
void interrupt_end(void)
{
+ struct pt_regs *regs = ¤t->thread.regs;
+
if (need_resched())
schedule();
if (test_thread_flag(TIF_SIGPENDING))
- do_signal();
+ do_signal(regs);
if (test_and_clear_thread_flag(TIF_NOTIFY_RESUME))
- tracehook_notify_resume(¤t->thread.regs);
+ tracehook_notify_resume(regs);
}
void exit_thread(void)
Index: tip/arch/um/kernel/signal.c
===================================================================
--- tip.orig/arch/um/kernel/signal.c
+++ tip/arch/um/kernel/signal.c
@@ -64,7 +64,7 @@ static void handle_signal(struct ksignal
signal_setup_done(err, ksig, singlestep);
}
-static int kern_do_signal(struct pt_regs *regs)
+void do_signal(struct pt_regs *regs)
{
struct ksignal ksig;
int handled_sig = 0;
@@ -110,10 +110,4 @@ static int kern_do_signal(struct pt_regs
*/
if (!handled_sig)
restore_saved_sigmask();
- return handled_sig;
-}
-
-int do_signal(void)
-{
- return kern_do_signal(¤t->thread.regs);
}
Index: tip/arch/um/kernel/tlb.c
===================================================================
--- tip.orig/arch/um/kernel/tlb.c
+++ tip/arch/um/kernel/tlb.c
@@ -291,7 +291,7 @@ void fix_range_common(struct mm_struct *
/* We are under mmap_sem, release it such that current can terminate */
up_write(¤t->mm->mmap_sem);
force_sig(SIGKILL, current);
- do_signal();
+ do_signal(¤t->thread.regs);
}
}
Index: tip/arch/um/kernel/trap.c
===================================================================
--- tip.orig/arch/um/kernel/trap.c
+++ tip/arch/um/kernel/trap.c
@@ -173,7 +173,7 @@ static void bad_segv(struct faultinfo fi
void fatal_sigsegv(void)
{
force_sigsegv(SIGSEGV, current);
- do_signal();
+ do_signal(¤t->thread.regs);
/*
* This is to tell gcc that we're not returning - do_signal
* can, in general, return, but in this case, it's not, since
next prev parent reply other threads:[~2015-06-17 11:05 UTC|newest]
Thread overview: 45+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-06-16 20:16 [RFC/INCOMPLETE 00/13] x86: Rewrite exit-to-userspace code Andy Lutomirski
2015-06-16 20:16 ` [RFC/INCOMPLETE 01/13] context_tracking: Add context_tracking_assert_state Andy Lutomirski
2015-06-17 9:41 ` Ingo Molnar
2015-06-17 14:15 ` Andy Lutomirski
2015-06-18 9:57 ` Ingo Molnar
2015-06-18 11:07 ` Andy Lutomirski
2015-06-18 15:52 ` Andy Lutomirski
2015-06-18 16:17 ` Ingo Molnar
2015-06-18 16:26 ` Frederic Weisbecker
2015-06-18 19:26 ` Andy Lutomirski
2015-06-17 15:27 ` Paul E. McKenney
2015-06-18 9:59 ` Ingo Molnar
2015-06-18 22:54 ` Paul E. McKenney
2015-06-19 2:19 ` Paul E. McKenney
2015-06-30 11:04 ` Ingo Molnar
2015-06-30 16:16 ` Paul E. McKenney
2015-06-16 20:16 ` [RFC/INCOMPLETE 02/13] notifiers: Assert that RCU is watching in notify_die Andy Lutomirski
2015-06-16 20:16 ` [RFC/INCOMPLETE 03/13] x86: Move C entry and exit code to arch/x86/entry/common.c Andy Lutomirski
2015-06-16 20:16 ` [RFC/INCOMPLETE 04/13] x86/traps: Assert that we're in CONTEXT_KERNEL in exception entries Andy Lutomirski
2015-06-16 20:16 ` [RFC/INCOMPLETE 05/13] x86/entry: Add enter_from_user_mode and use it in syscalls Andy Lutomirski
2015-06-16 20:16 ` [RFC/INCOMPLETE 06/13] x86/entry: Add new, comprehensible entry and exit hooks Andy Lutomirski
2015-06-16 20:16 ` [RFC/INCOMPLETE 07/13] x86/entry/64: Really create an error-entry-from-usermode code path Andy Lutomirski
2015-06-16 20:16 ` [RFC/INCOMPLETE 08/13] x86/entry/64: Migrate 64-bit syscalls to new exit hooks Andy Lutomirski
2015-06-17 10:00 ` Ingo Molnar
2015-06-17 10:02 ` Ingo Molnar
2015-06-17 14:12 ` Andy Lutomirski
2015-06-18 10:17 ` Ingo Molnar
2015-06-18 10:19 ` Ingo Molnar
2015-06-16 20:16 ` [RFC/INCOMPLETE 09/13] x86/entry/compat: Migrate compat " Andy Lutomirski
2015-06-16 20:16 ` [RFC/INCOMPLETE 10/13] x86/asm/entry/64: Save all regs on interrupt entry Andy Lutomirski
2015-06-16 20:16 ` [RFC/INCOMPLETE 11/13] x86/asm/entry/64: Simplify irq stack pt_regs handling Andy Lutomirski
2015-06-16 20:16 ` [RFC/INCOMPLETE 12/13] x86/asm/entry/64: Migrate error and interrupt exit work to C Andy Lutomirski
2015-06-16 20:16 ` [RFC/INCOMPLETE 13/13] x86/entry: Remove SCHEDULE_USER and asm/context-tracking.h Andy Lutomirski
2015-06-17 9:48 ` [RFC/INCOMPLETE 00/13] x86: Rewrite exit-to-userspace code Ingo Molnar
2015-06-17 10:13 ` Richard Weinberger
2015-06-17 11:04 ` Ingo Molnar [this message]
2015-06-17 14:19 ` Andy Lutomirski
2015-06-17 15:16 ` Andy Lutomirski
2015-06-18 10:14 ` Ingo Molnar
2015-06-17 10:32 ` Ingo Molnar
2015-06-17 11:14 ` Ingo Molnar
2015-06-17 14:23 ` Andy Lutomirski
2015-06-18 10:11 ` Ingo Molnar
2015-06-18 11:06 ` Andy Lutomirski
2015-06-18 16:24 ` 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=20150617110450.GA8919@gmail.com \
--to=mingo@kernel.org \
--cc=bp@alien8.de \
--cc=brgerst@gmail.com \
--cc=dvlasenk@redhat.com \
--cc=fweisbec@gmail.com \
--cc=keescook@chromium.org \
--cc=linux-kernel@vger.kernel.org \
--cc=luto@kernel.org \
--cc=oleg@redhat.com \
--cc=richard.weinberger@gmail.com \
--cc=riel@redhat.com \
--cc=torvalds@linux-foundation.org \
--cc=vda.linux@googlemail.com \
--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.