Rename syscall_exit_prepare_main() into interrupt_exit_prepare_main()
Make it static as it is not used anywhere else.
Pass it the 'ret' so that it can 'or' it directly instead of
oring twice, once inside the function and once outside.
And remove 'r3' parameter which is not used.
Also fix a typo where CONFIG_PPC_BOOK3S should be CONFIG_PPC_BOOK3S_64.
Signed-off-by: Christophe Leroy <[email protected]>
Reviewed-by: Nicholas Piggin <[email protected]>
---
This series applies on top of Nic's series speeding up interrupt return on 64s
arch/powerpc/kernel/interrupt.c | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/arch/powerpc/kernel/interrupt.c b/arch/powerpc/kernel/interrupt.c
index 74c995a42399..ba2d602d2da6 100644
--- a/arch/powerpc/kernel/interrupt.c
+++ b/arch/powerpc/kernel/interrupt.c
@@ -243,11 +243,10 @@ static notrace void booke_load_dbcr0(void)
#endif
}
-notrace unsigned long syscall_exit_prepare_main(unsigned long r3,
- struct pt_regs *regs)
+static notrace unsigned long
+interrupt_exit_user_prepare_main(struct pt_regs *regs, unsigned long ret)
{
unsigned long ti_flags;
- unsigned long ret = 0;
again:
ti_flags = READ_ONCE(current_thread_info()->flags);
@@ -269,7 +268,7 @@ notrace unsigned long syscall_exit_prepare_main(unsigned long r3,
ti_flags = READ_ONCE(current_thread_info()->flags);
}
- if (IS_ENABLED(CONFIG_PPC_BOOK3S) && IS_ENABLED(CONFIG_PPC_FPU)) {
+ if (IS_ENABLED(CONFIG_PPC_BOOK3S_64) && IS_ENABLED(CONFIG_PPC_FPU)) {
if (IS_ENABLED(CONFIG_PPC_TRANSACTIONAL_MEM) &&
unlikely((ti_flags & _TIF_RESTORE_TM))) {
restore_tm_state(regs);
@@ -365,7 +364,7 @@ notrace unsigned long syscall_exit_prepare(unsigned long r3,
}
local_irq_disable();
- ret |= syscall_exit_prepare_main(r3, regs);
+ ret = interrupt_exit_user_prepare_main(regs, ret);
#ifdef CONFIG_PPC64
regs->exit_result = ret;
@@ -393,7 +392,7 @@ notrace unsigned long syscall_exit_restart(unsigned long r3, struct pt_regs *reg
BUG_ON(!user_mode(regs));
- regs->exit_result |= syscall_exit_prepare_main(r3, regs);
+ regs->exit_result = interrupt_exit_user_prepare_main(regs, regs->exit_result);
return regs->exit_result;
}
--
2.25.0
interrupt_exit_user_prepare() is a superset of
interrupt_exit_user_prepare_main().
Refactor to avoid code duplication.
Signed-off-by: Christophe Leroy <[email protected]>
---
arch/powerpc/kernel/interrupt.c | 57 ++-------------------------------
1 file changed, 3 insertions(+), 54 deletions(-)
diff --git a/arch/powerpc/kernel/interrupt.c b/arch/powerpc/kernel/interrupt.c
index ba2d602d2da6..b9558372adc0 100644
--- a/arch/powerpc/kernel/interrupt.c
+++ b/arch/powerpc/kernel/interrupt.c
@@ -400,9 +400,7 @@ notrace unsigned long syscall_exit_restart(unsigned long r3, struct pt_regs *reg
notrace unsigned long interrupt_exit_user_prepare(struct pt_regs *regs)
{
- unsigned long ti_flags;
- unsigned long flags;
- unsigned long ret = 0;
+ unsigned long ret;
if (!IS_ENABLED(CONFIG_BOOKE) && !IS_ENABLED(CONFIG_40x))
BUG_ON(!(regs->msr & MSR_RI));
@@ -416,63 +414,14 @@ notrace unsigned long interrupt_exit_user_prepare(struct pt_regs *regs)
*/
kuap_assert_locked();
- local_irq_save(flags);
-
-again:
- ti_flags = READ_ONCE(current_thread_info()->flags);
- while (unlikely(ti_flags & (_TIF_USER_WORK_MASK & ~_TIF_RESTORE_TM))) {
- local_irq_enable(); /* returning to user: may enable */
- if (ti_flags & _TIF_NEED_RESCHED) {
- schedule();
- } else {
- if (ti_flags & _TIF_SIGPENDING)
- ret |= _TIF_RESTOREALL;
- do_notify_resume(regs, ti_flags);
- }
- local_irq_disable();
- ti_flags = READ_ONCE(current_thread_info()->flags);
- }
-
- if (IS_ENABLED(CONFIG_PPC_BOOK3S_64) && IS_ENABLED(CONFIG_PPC_FPU)) {
- if (IS_ENABLED(CONFIG_PPC_TRANSACTIONAL_MEM) &&
- unlikely((ti_flags & _TIF_RESTORE_TM))) {
- restore_tm_state(regs);
- } else {
- unsigned long mathflags = MSR_FP;
-
- if (cpu_has_feature(CPU_FTR_VSX))
- mathflags |= MSR_VEC | MSR_VSX;
- else if (cpu_has_feature(CPU_FTR_ALTIVEC))
- mathflags |= MSR_VEC;
-
- /* See above restore_math comment */
- if ((regs->msr & mathflags) != mathflags)
- restore_math(regs);
- }
- }
-
- if (!prep_irq_for_user_exit()) {
- local_irq_enable();
- local_irq_disable();
- goto again;
- }
-
- booke_load_dbcr0();
-
-#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
- local_paca->tm_scratch = regs->msr;
-#endif
+ local_irq_disable();
- account_cpu_user_exit();
+ ret = interrupt_exit_user_prepare_main(regs, 0);
#ifdef CONFIG_PPC64
regs->exit_result = ret;
#endif
- /* Restore user access locks last */
- kuap_user_restore(regs);
- kuep_unlock();
-
return ret;
}
--
2.25.0
prep_irq_for_user_exit() has only one caller, squash it
inside that caller.
Signed-off-by: Christophe Leroy <[email protected]>
---
arch/powerpc/kernel/interrupt.c | 16 +++-------------
1 file changed, 3 insertions(+), 13 deletions(-)
diff --git a/arch/powerpc/kernel/interrupt.c b/arch/powerpc/kernel/interrupt.c
index 05831d99bf26..de335da7ab52 100644
--- a/arch/powerpc/kernel/interrupt.c
+++ b/arch/powerpc/kernel/interrupt.c
@@ -75,18 +75,6 @@ static notrace __always_inline bool prep_irq_for_enabled_exit(bool restartable)
return true;
}
-static notrace __always_inline bool prep_irq_for_user_exit(void)
-{
- bool ret;
-
- user_enter_irqoff();
- ret = prep_irq_for_enabled_exit(true);
- if (!ret)
- user_exit_irqoff();
-
- return ret;
-}
-
/* Has to run notrace because it is entered not completely "reconciled" */
notrace long system_call_exception(long r3, long r4, long r5,
long r6, long r7, long r8,
@@ -276,7 +264,9 @@ interrupt_exit_user_prepare_main(struct pt_regs *regs, unsigned long ret)
}
}
- if (!prep_irq_for_user_exit()) {
+ user_enter_irqoff();
+ if (!prep_irq_for_enabled_exit(true)) {
+ user_exit_irqoff();
local_irq_enable();
local_irq_disable();
goto again;
--
2.25.0
prep_irq_for_user_exit() is a superset of
prep_irq_for_kernel_enabled_exit(). In order to allow refactoring in
following patch, interchange the two. This will allow
prep_irq_for_user_exit() to call a renamed version of
prep_irq_for_kernel_enabled_exit().
Signed-off-by: Christophe Leroy <[email protected]>
Reviewed-by: Nicholas Piggin <[email protected]>
---
arch/powerpc/kernel/interrupt.c | 23 +++++++++++------------
1 file changed, 11 insertions(+), 12 deletions(-)
diff --git a/arch/powerpc/kernel/interrupt.c b/arch/powerpc/kernel/interrupt.c
index b9558372adc0..9780c26f19cf 100644
--- a/arch/powerpc/kernel/interrupt.c
+++ b/arch/powerpc/kernel/interrupt.c
@@ -46,27 +46,28 @@ static inline bool exit_must_hard_disable(void)
* This should be called with local irqs disabled, but if they were previously
* enabled when the interrupt handler returns (indicating a process-context /
* synchronous interrupt) then irqs_enabled should be true.
+ *
+ * restartable is true then EE/RI can be left on because interrupts are handled
+ * with a restart sequence.
*/
-static notrace __always_inline bool prep_irq_for_user_exit(void)
+static notrace __always_inline bool prep_irq_for_kernel_enabled_exit(bool restartable)
{
- user_enter_irqoff();
/* This must be done with RI=1 because tracing may touch vmaps */
trace_hardirqs_on();
#ifdef CONFIG_PPC32
__hard_EE_RI_disable();
#else
- if (exit_must_hard_disable())
+ if (exit_must_hard_disable() || !restartable)
__hard_EE_RI_disable();
/* This pattern matches prep_irq_for_idle */
if (unlikely(lazy_irq_pending_nocheck())) {
- if (exit_must_hard_disable()) {
+ if (exit_must_hard_disable() || !restartable) {
local_paca->irq_happened |= PACA_IRQ_HARD_DIS;
__hard_RI_enable();
}
trace_hardirqs_off();
- user_exit_irqoff();
return false;
}
@@ -74,28 +75,26 @@ static notrace __always_inline bool prep_irq_for_user_exit(void)
return true;
}
-/*
- * restartable is true then EE/RI can be left on because interrupts are handled
- * with a restart sequence.
- */
-static notrace __always_inline bool prep_irq_for_kernel_enabled_exit(bool restartable)
+static notrace __always_inline bool prep_irq_for_user_exit(void)
{
+ user_enter_irqoff();
/* This must be done with RI=1 because tracing may touch vmaps */
trace_hardirqs_on();
#ifdef CONFIG_PPC32
__hard_EE_RI_disable();
#else
- if (exit_must_hard_disable() || !restartable)
+ if (exit_must_hard_disable())
__hard_EE_RI_disable();
/* This pattern matches prep_irq_for_idle */
if (unlikely(lazy_irq_pending_nocheck())) {
- if (exit_must_hard_disable() || !restartable) {
+ if (exit_must_hard_disable()) {
local_paca->irq_happened |= PACA_IRQ_HARD_DIS;
__hard_RI_enable();
}
trace_hardirqs_off();
+ user_exit_irqoff();
return false;
}
--
2.25.0
Excerpts from Christophe Leroy's message of June 15, 2021 6:33 pm:
> Rename syscall_exit_prepare_main() into interrupt_exit_prepare_main()
>
> Make it static as it is not used anywhere else.
>
> Pass it the 'ret' so that it can 'or' it directly instead of
> oring twice, once inside the function and once outside.
>
> And remove 'r3' parameter which is not used.
>
> Also fix a typo where CONFIG_PPC_BOOK3S should be CONFIG_PPC_BOOK3S_64.
>
> Signed-off-by: Christophe Leroy <[email protected]>
> Reviewed-by: Nicholas Piggin <[email protected]>
> ---
> This series applies on top of Nic's series speeding up interrupt return on 64s
>
> arch/powerpc/kernel/interrupt.c | 11 +++++------
> 1 file changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/arch/powerpc/kernel/interrupt.c b/arch/powerpc/kernel/interrupt.c
> index 74c995a42399..ba2d602d2da6 100644
> --- a/arch/powerpc/kernel/interrupt.c
> +++ b/arch/powerpc/kernel/interrupt.c
> @@ -243,11 +243,10 @@ static notrace void booke_load_dbcr0(void)
> #endif
> }
>
> -notrace unsigned long syscall_exit_prepare_main(unsigned long r3,
> - struct pt_regs *regs)
> +static notrace unsigned long
> +interrupt_exit_user_prepare_main(struct pt_regs *regs, unsigned long ret)
Hmm, I tried switching the order of the arguments thinking it would
match caller and return registers better but didn't seem to help
generated code. Yet I think I will make that change to your patch if
you don't mind.
Thanks,
Nick
Le 17/06/2021 à 13:25, Nicholas Piggin a écrit :
> Excerpts from Christophe Leroy's message of June 15, 2021 6:33 pm:
>> Rename syscall_exit_prepare_main() into interrupt_exit_prepare_main()
>>
>> Make it static as it is not used anywhere else.
>>
>> Pass it the 'ret' so that it can 'or' it directly instead of
>> oring twice, once inside the function and once outside.
>>
>> And remove 'r3' parameter which is not used.
>>
>> Also fix a typo where CONFIG_PPC_BOOK3S should be CONFIG_PPC_BOOK3S_64.
>>
>> Signed-off-by: Christophe Leroy <[email protected]>
>> Reviewed-by: Nicholas Piggin <[email protected]>
>> ---
>> This series applies on top of Nic's series speeding up interrupt return on 64s
>>
>> arch/powerpc/kernel/interrupt.c | 11 +++++------
>> 1 file changed, 5 insertions(+), 6 deletions(-)
>>
>> diff --git a/arch/powerpc/kernel/interrupt.c b/arch/powerpc/kernel/interrupt.c
>> index 74c995a42399..ba2d602d2da6 100644
>> --- a/arch/powerpc/kernel/interrupt.c
>> +++ b/arch/powerpc/kernel/interrupt.c
>> @@ -243,11 +243,10 @@ static notrace void booke_load_dbcr0(void)
>> #endif
>> }
>>
>> -notrace unsigned long syscall_exit_prepare_main(unsigned long r3,
>> - struct pt_regs *regs)
>> +static notrace unsigned long
>> +interrupt_exit_user_prepare_main(struct pt_regs *regs, unsigned long ret)
>
> Hmm, I tried switching the order of the arguments thinking it would
> match caller and return registers better but didn't seem to help
> generated code. Yet I think I will make that change to your patch if
> you don't mind.
That's a static function that most likely gets inlined so the order of parameters makes no difference.
I tend to like that almost all functions dealing with interrupts take regs as first param, but I
have no strong opinion about it so you can change it if that's better for you.
Christophe