Re: [PATCH v3 07/11] kentry: Make entry/exit_to_user_mode() arm64-only
On Mon, Mar 8, 2021 at 2:06 AM Mark Rutland wrote: > > On Thu, Mar 04, 2021 at 11:06:00AM -0800, Andy Lutomirski wrote: > > exit_to_user_mode() does part, but not all, of the exit-to-user-mode work. > > It's used only by arm64, and arm64 should stop using it (hint, hint!). > > Compile it out on other architectures to minimize the chance of error. > > For context, I do plan to move over, but there's a reasonable amount of > preparatory work needed first (e.g. factoring out the remaining asm > entry points, and reworking our pseudo-NMI management to play nicely > with the common entry code). > > > enter_from_user_mode() is a legacy way to spell > > kentry_enter_from_user_mode(). It's also only used by arm64. Give it > > the same treatment. > > I think you can remove these entirely, no ifdeferry necessary. > > Currently arm64 cannot select CONFIG_GENERIC_ENTRY, so we open-code > copies of these in arch/arm64/kernel/entry-common.c, and doesn't use > these common versions at all. When we move over to the common code we > can move directly to the kentry_* versions. If we are relying on the > prototypes anywhere, that's a bug as of itself. I got faked out! Fixed for the next version.
Re: [PATCH v3 07/11] kentry: Make entry/exit_to_user_mode() arm64-only
On Thu, Mar 04, 2021 at 11:06:00AM -0800, Andy Lutomirski wrote: > exit_to_user_mode() does part, but not all, of the exit-to-user-mode work. > It's used only by arm64, and arm64 should stop using it (hint, hint!). > Compile it out on other architectures to minimize the chance of error. For context, I do plan to move over, but there's a reasonable amount of preparatory work needed first (e.g. factoring out the remaining asm entry points, and reworking our pseudo-NMI management to play nicely with the common entry code). > enter_from_user_mode() is a legacy way to spell > kentry_enter_from_user_mode(). It's also only used by arm64. Give it > the same treatment. I think you can remove these entirely, no ifdeferry necessary. Currently arm64 cannot select CONFIG_GENERIC_ENTRY, so we open-code copies of these in arch/arm64/kernel/entry-common.c, and doesn't use these common versions at all. When we move over to the common code we can move directly to the kentry_* versions. If we are relying on the prototypes anywhere, that's a bug as of itself. In retrospect I probably should have given our local copies an arm64_* prefix. If I can't get rid of them soon I'll add that to lessen the scope for confusion. Mark. > Signed-off-by: Andy Lutomirski > --- > include/linux/entry-common.h | 34 ++ > kernel/entry/common.c| 4 > 2 files changed, 10 insertions(+), 28 deletions(-) > > diff --git a/include/linux/entry-common.h b/include/linux/entry-common.h > index 5287c6c15a66..a75374f87258 100644 > --- a/include/linux/entry-common.h > +++ b/include/linux/entry-common.h > @@ -97,26 +97,15 @@ static inline __must_check int > arch_syscall_enter_tracehook(struct pt_regs *regs > } > #endif > > +#ifdef CONFIG_ARM64 > /** > * enter_from_user_mode - Establish state when coming from user mode > * > - * Syscall/interrupt entry disables interrupts, but user mode is traced as > - * interrupts enabled. Also with NO_HZ_FULL RCU might be idle. > + * Legacy variant of kentry_enter_from_user_mode(). Used only by arm64. > * > - * 1) Tell lockdep that interrupts are disabled > - * 2) Invoke context tracking if enabled to reactivate RCU > - * 3) Trace interrupts off state > - * > - * Invoked from architecture specific syscall entry code with interrupts > - * disabled. The calling code has to be non-instrumentable. When the > - * function returns all state is correct and interrupts are still > - * disabled. The subsequent functions can be instrumented. > - * > - * This is invoked when there is architecture specific functionality to be > - * done between establishing state and enabling interrupts. The caller must > - * enable interrupts before invoking syscall_enter_from_user_mode_work(). > */ > void enter_from_user_mode(struct pt_regs *regs); > +#endif > > /** > * kentry_syscall_begin - Prepare to invoke a syscall handler > @@ -261,25 +250,14 @@ static inline void arch_syscall_exit_tracehook(struct > pt_regs *regs, bool step) > } > #endif > > +#ifdef CONFIG_ARM64 > /** > * exit_to_user_mode - Fixup state when exiting to user mode > * > - * Syscall/interrupt exit enables interrupts, but the kernel state is > - * interrupts disabled when this is invoked. Also tell RCU about it. > - * > - * 1) Trace interrupts on state > - * 2) Invoke context tracking if enabled to adjust RCU state > - * 3) Invoke architecture specific last minute exit code, e.g. speculation > - *mitigations, etc.: arch_exit_to_user_mode() > - * 4) Tell lockdep that interrupts are enabled > - * > - * Invoked from architecture specific code when syscall_exit_to_user_mode() > - * is not suitable as the last step before returning to userspace. Must be > - * invoked with interrupts disabled and the caller must be > - * non-instrumentable. > - * The caller has to invoke syscall_exit_to_user_mode_work() before this. > + * Does the latter part of irqentry_exit_to_user_mode(). Only used by arm64. > */ > void exit_to_user_mode(void); > +#endif > > /** > * kentry_syscall_end - Finish syscall processing > diff --git a/kernel/entry/common.c b/kernel/entry/common.c > index 800ad406431b..4ba82c684189 100644 > --- a/kernel/entry/common.c > +++ b/kernel/entry/common.c > @@ -25,10 +25,12 @@ static __always_inline void __enter_from_user_mode(struct > pt_regs *regs) > instrumentation_end(); > } > > +#ifdef CONFIG_ARM64 > void noinstr enter_from_user_mode(struct pt_regs *regs) > { > __enter_from_user_mode(regs); > } > +#endif > > static inline void syscall_enter_audit(struct pt_regs *regs, long syscall) > { > @@ -106,10 +108,12 @@ static __always_inline void __exit_to_user_mode(void) > lockdep_hardirqs_on(CALLER_ADDR0); > } > > +#ifdef CONFIG_ARM64 > void noinstr exit_to_user_mode(void) > { > __exit_to_user_mode(); > } > +#endif > > /* Workaround to allow gradual conversion of architecture code */ > void __weak arch_do_signal_or_restart(struct pt_regs *regs,
[PATCH v3 07/11] kentry: Make entry/exit_to_user_mode() arm64-only
exit_to_user_mode() does part, but not all, of the exit-to-user-mode work. It's used only by arm64, and arm64 should stop using it (hint, hint!). Compile it out on other architectures to minimize the chance of error. enter_from_user_mode() is a legacy way to spell kentry_enter_from_user_mode(). It's also only used by arm64. Give it the same treatment. Signed-off-by: Andy Lutomirski --- include/linux/entry-common.h | 34 ++ kernel/entry/common.c| 4 2 files changed, 10 insertions(+), 28 deletions(-) diff --git a/include/linux/entry-common.h b/include/linux/entry-common.h index 5287c6c15a66..a75374f87258 100644 --- a/include/linux/entry-common.h +++ b/include/linux/entry-common.h @@ -97,26 +97,15 @@ static inline __must_check int arch_syscall_enter_tracehook(struct pt_regs *regs } #endif +#ifdef CONFIG_ARM64 /** * enter_from_user_mode - Establish state when coming from user mode * - * Syscall/interrupt entry disables interrupts, but user mode is traced as - * interrupts enabled. Also with NO_HZ_FULL RCU might be idle. + * Legacy variant of kentry_enter_from_user_mode(). Used only by arm64. * - * 1) Tell lockdep that interrupts are disabled - * 2) Invoke context tracking if enabled to reactivate RCU - * 3) Trace interrupts off state - * - * Invoked from architecture specific syscall entry code with interrupts - * disabled. The calling code has to be non-instrumentable. When the - * function returns all state is correct and interrupts are still - * disabled. The subsequent functions can be instrumented. - * - * This is invoked when there is architecture specific functionality to be - * done between establishing state and enabling interrupts. The caller must - * enable interrupts before invoking syscall_enter_from_user_mode_work(). */ void enter_from_user_mode(struct pt_regs *regs); +#endif /** * kentry_syscall_begin - Prepare to invoke a syscall handler @@ -261,25 +250,14 @@ static inline void arch_syscall_exit_tracehook(struct pt_regs *regs, bool step) } #endif +#ifdef CONFIG_ARM64 /** * exit_to_user_mode - Fixup state when exiting to user mode * - * Syscall/interrupt exit enables interrupts, but the kernel state is - * interrupts disabled when this is invoked. Also tell RCU about it. - * - * 1) Trace interrupts on state - * 2) Invoke context tracking if enabled to adjust RCU state - * 3) Invoke architecture specific last minute exit code, e.g. speculation - *mitigations, etc.: arch_exit_to_user_mode() - * 4) Tell lockdep that interrupts are enabled - * - * Invoked from architecture specific code when syscall_exit_to_user_mode() - * is not suitable as the last step before returning to userspace. Must be - * invoked with interrupts disabled and the caller must be - * non-instrumentable. - * The caller has to invoke syscall_exit_to_user_mode_work() before this. + * Does the latter part of irqentry_exit_to_user_mode(). Only used by arm64. */ void exit_to_user_mode(void); +#endif /** * kentry_syscall_end - Finish syscall processing diff --git a/kernel/entry/common.c b/kernel/entry/common.c index 800ad406431b..4ba82c684189 100644 --- a/kernel/entry/common.c +++ b/kernel/entry/common.c @@ -25,10 +25,12 @@ static __always_inline void __enter_from_user_mode(struct pt_regs *regs) instrumentation_end(); } +#ifdef CONFIG_ARM64 void noinstr enter_from_user_mode(struct pt_regs *regs) { __enter_from_user_mode(regs); } +#endif static inline void syscall_enter_audit(struct pt_regs *regs, long syscall) { @@ -106,10 +108,12 @@ static __always_inline void __exit_to_user_mode(void) lockdep_hardirqs_on(CALLER_ADDR0); } +#ifdef CONFIG_ARM64 void noinstr exit_to_user_mode(void) { __exit_to_user_mode(); } +#endif /* Workaround to allow gradual conversion of architecture code */ void __weak arch_do_signal_or_restart(struct pt_regs *regs, bool has_signal) { } -- 2.29.2