* [PATCH] powerpc: Remove MSR_PR check in interrupt_exit_{user/kernel}_prepare()
@ 2021-08-04 5:27 ` Christophe Leroy
0 siblings, 0 replies; 8+ messages in thread
From: Christophe Leroy @ 2021-08-04 5:27 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman, npiggin
Cc: linux-kernel, linuxppc-dev
In those hot functions that are called at every interrupt, any saved
cycle is worth it.
interrupt_exit_user_prepare() and interrupt_exit_kernel_prepare() are
called from three places:
- From entry_32.S
- From interrupt_64.S
- From interrupt_exit_user_restart() and interrupt_exit_kernel_restart()
In entry_32.S, there are inambiguously called based on MSR_PR:
interrupt_return:
lwz r4,_MSR(r1)
addi r3,r1,STACK_FRAME_OVERHEAD
andi. r0,r4,MSR_PR
beq .Lkernel_interrupt_return
bl interrupt_exit_user_prepare
...
.Lkernel_interrupt_return:
bl interrupt_exit_kernel_prepare
In interrupt_64.S, that's similar:
interrupt_return_\srr\():
ld r4,_MSR(r1)
andi. r0,r4,MSR_PR
beq interrupt_return_\srr\()_kernel
interrupt_return_\srr\()_user: /* make backtraces match the _kernel variant */
addi r3,r1,STACK_FRAME_OVERHEAD
bl interrupt_exit_user_prepare
...
interrupt_return_\srr\()_kernel:
addi r3,r1,STACK_FRAME_OVERHEAD
bl interrupt_exit_kernel_prepare
In interrupt_exit_user_restart() and interrupt_exit_kernel_restart(),
MSR_PR is verified respectively by BUG_ON(!user_mode(regs)) and
BUG_ON(user_mode(regs)) prior to calling interrupt_exit_user_prepare()
and interrupt_exit_kernel_prepare().
The verification in interrupt_exit_user_prepare() and
interrupt_exit_kernel_prepare() are therefore useless and can be removed.
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
arch/powerpc/kernel/interrupt.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/arch/powerpc/kernel/interrupt.c b/arch/powerpc/kernel/interrupt.c
index 21bbd615ca41..f26caf911ab5 100644
--- a/arch/powerpc/kernel/interrupt.c
+++ b/arch/powerpc/kernel/interrupt.c
@@ -465,7 +465,6 @@ notrace unsigned long interrupt_exit_user_prepare(struct pt_regs *regs)
if (!IS_ENABLED(CONFIG_BOOKE) && !IS_ENABLED(CONFIG_40x))
BUG_ON(!(regs->msr & MSR_RI));
- BUG_ON(!(regs->msr & MSR_PR));
BUG_ON(arch_irq_disabled_regs(regs));
CT_WARN_ON(ct_state() == CONTEXT_USER);
@@ -499,7 +498,6 @@ notrace unsigned long interrupt_exit_kernel_prepare(struct pt_regs *regs)
if (!IS_ENABLED(CONFIG_BOOKE) && !IS_ENABLED(CONFIG_40x) &&
unlikely(!(regs->msr & MSR_RI)))
unrecoverable_exception(regs);
- BUG_ON(regs->msr & MSR_PR);
/*
* CT_WARN_ON comes here via program_check_exception,
* so avoid recursion.
--
2.25.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH] powerpc: Remove MSR_PR check in interrupt_exit_{user/kernel}_prepare()
@ 2021-08-04 5:27 ` Christophe Leroy
0 siblings, 0 replies; 8+ messages in thread
From: Christophe Leroy @ 2021-08-04 5:27 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman, npiggin
Cc: linuxppc-dev, linux-kernel
In those hot functions that are called at every interrupt, any saved
cycle is worth it.
interrupt_exit_user_prepare() and interrupt_exit_kernel_prepare() are
called from three places:
- From entry_32.S
- From interrupt_64.S
- From interrupt_exit_user_restart() and interrupt_exit_kernel_restart()
In entry_32.S, there are inambiguously called based on MSR_PR:
interrupt_return:
lwz r4,_MSR(r1)
addi r3,r1,STACK_FRAME_OVERHEAD
andi. r0,r4,MSR_PR
beq .Lkernel_interrupt_return
bl interrupt_exit_user_prepare
...
.Lkernel_interrupt_return:
bl interrupt_exit_kernel_prepare
In interrupt_64.S, that's similar:
interrupt_return_\srr\():
ld r4,_MSR(r1)
andi. r0,r4,MSR_PR
beq interrupt_return_\srr\()_kernel
interrupt_return_\srr\()_user: /* make backtraces match the _kernel variant */
addi r3,r1,STACK_FRAME_OVERHEAD
bl interrupt_exit_user_prepare
...
interrupt_return_\srr\()_kernel:
addi r3,r1,STACK_FRAME_OVERHEAD
bl interrupt_exit_kernel_prepare
In interrupt_exit_user_restart() and interrupt_exit_kernel_restart(),
MSR_PR is verified respectively by BUG_ON(!user_mode(regs)) and
BUG_ON(user_mode(regs)) prior to calling interrupt_exit_user_prepare()
and interrupt_exit_kernel_prepare().
The verification in interrupt_exit_user_prepare() and
interrupt_exit_kernel_prepare() are therefore useless and can be removed.
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
arch/powerpc/kernel/interrupt.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/arch/powerpc/kernel/interrupt.c b/arch/powerpc/kernel/interrupt.c
index 21bbd615ca41..f26caf911ab5 100644
--- a/arch/powerpc/kernel/interrupt.c
+++ b/arch/powerpc/kernel/interrupt.c
@@ -465,7 +465,6 @@ notrace unsigned long interrupt_exit_user_prepare(struct pt_regs *regs)
if (!IS_ENABLED(CONFIG_BOOKE) && !IS_ENABLED(CONFIG_40x))
BUG_ON(!(regs->msr & MSR_RI));
- BUG_ON(!(regs->msr & MSR_PR));
BUG_ON(arch_irq_disabled_regs(regs));
CT_WARN_ON(ct_state() == CONTEXT_USER);
@@ -499,7 +498,6 @@ notrace unsigned long interrupt_exit_kernel_prepare(struct pt_regs *regs)
if (!IS_ENABLED(CONFIG_BOOKE) && !IS_ENABLED(CONFIG_40x) &&
unlikely(!(regs->msr & MSR_RI)))
unrecoverable_exception(regs);
- BUG_ON(regs->msr & MSR_PR);
/*
* CT_WARN_ON comes here via program_check_exception,
* so avoid recursion.
--
2.25.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH] powerpc: Remove MSR_PR check in interrupt_exit_{user/kernel}_prepare()
2021-08-04 5:27 ` Christophe Leroy
@ 2021-08-04 8:08 ` Nicholas Piggin
-1 siblings, 0 replies; 8+ messages in thread
From: Nicholas Piggin @ 2021-08-04 8:08 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Christophe Leroy, Michael Ellerman,
Paul Mackerras
Cc: linux-kernel, linuxppc-dev
Excerpts from Christophe Leroy's message of August 4, 2021 3:27 pm:
> In those hot functions that are called at every interrupt, any saved
> cycle is worth it.
>
> interrupt_exit_user_prepare() and interrupt_exit_kernel_prepare() are
> called from three places:
> - From entry_32.S
> - From interrupt_64.S
> - From interrupt_exit_user_restart() and interrupt_exit_kernel_restart()
>
> In entry_32.S, there are inambiguously called based on MSR_PR:
>
> interrupt_return:
> lwz r4,_MSR(r1)
> addi r3,r1,STACK_FRAME_OVERHEAD
> andi. r0,r4,MSR_PR
> beq .Lkernel_interrupt_return
> bl interrupt_exit_user_prepare
> ...
> .Lkernel_interrupt_return:
> bl interrupt_exit_kernel_prepare
>
> In interrupt_64.S, that's similar:
>
> interrupt_return_\srr\():
> ld r4,_MSR(r1)
> andi. r0,r4,MSR_PR
> beq interrupt_return_\srr\()_kernel
> interrupt_return_\srr\()_user: /* make backtraces match the _kernel variant */
> addi r3,r1,STACK_FRAME_OVERHEAD
> bl interrupt_exit_user_prepare
> ...
> interrupt_return_\srr\()_kernel:
> addi r3,r1,STACK_FRAME_OVERHEAD
> bl interrupt_exit_kernel_prepare
>
> In interrupt_exit_user_restart() and interrupt_exit_kernel_restart(),
> MSR_PR is verified respectively by BUG_ON(!user_mode(regs)) and
> BUG_ON(user_mode(regs)) prior to calling interrupt_exit_user_prepare()
> and interrupt_exit_kernel_prepare().
>
> The verification in interrupt_exit_user_prepare() and
> interrupt_exit_kernel_prepare() are therefore useless and can be removed.
>
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
Probably okay to do now things are ironing out.
Unless we want to make a new define for interrupt handler debug and put
a bunch of these asserts under it. There's quite a lot more here, and
in asm/interrupt.h, etc.
Thanks,
Nick
> ---
> arch/powerpc/kernel/interrupt.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/arch/powerpc/kernel/interrupt.c b/arch/powerpc/kernel/interrupt.c
> index 21bbd615ca41..f26caf911ab5 100644
> --- a/arch/powerpc/kernel/interrupt.c
> +++ b/arch/powerpc/kernel/interrupt.c
> @@ -465,7 +465,6 @@ notrace unsigned long interrupt_exit_user_prepare(struct pt_regs *regs)
>
> if (!IS_ENABLED(CONFIG_BOOKE) && !IS_ENABLED(CONFIG_40x))
> BUG_ON(!(regs->msr & MSR_RI));
> - BUG_ON(!(regs->msr & MSR_PR));
> BUG_ON(arch_irq_disabled_regs(regs));
> CT_WARN_ON(ct_state() == CONTEXT_USER);
>
> @@ -499,7 +498,6 @@ notrace unsigned long interrupt_exit_kernel_prepare(struct pt_regs *regs)
> if (!IS_ENABLED(CONFIG_BOOKE) && !IS_ENABLED(CONFIG_40x) &&
> unlikely(!(regs->msr & MSR_RI)))
> unrecoverable_exception(regs);
> - BUG_ON(regs->msr & MSR_PR);
> /*
> * CT_WARN_ON comes here via program_check_exception,
> * so avoid recursion.
> --
> 2.25.0
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] powerpc: Remove MSR_PR check in interrupt_exit_{user/kernel}_prepare()
@ 2021-08-04 8:08 ` Nicholas Piggin
0 siblings, 0 replies; 8+ messages in thread
From: Nicholas Piggin @ 2021-08-04 8:08 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Christophe Leroy, Michael Ellerman,
Paul Mackerras
Cc: linuxppc-dev, linux-kernel
Excerpts from Christophe Leroy's message of August 4, 2021 3:27 pm:
> In those hot functions that are called at every interrupt, any saved
> cycle is worth it.
>
> interrupt_exit_user_prepare() and interrupt_exit_kernel_prepare() are
> called from three places:
> - From entry_32.S
> - From interrupt_64.S
> - From interrupt_exit_user_restart() and interrupt_exit_kernel_restart()
>
> In entry_32.S, there are inambiguously called based on MSR_PR:
>
> interrupt_return:
> lwz r4,_MSR(r1)
> addi r3,r1,STACK_FRAME_OVERHEAD
> andi. r0,r4,MSR_PR
> beq .Lkernel_interrupt_return
> bl interrupt_exit_user_prepare
> ...
> .Lkernel_interrupt_return:
> bl interrupt_exit_kernel_prepare
>
> In interrupt_64.S, that's similar:
>
> interrupt_return_\srr\():
> ld r4,_MSR(r1)
> andi. r0,r4,MSR_PR
> beq interrupt_return_\srr\()_kernel
> interrupt_return_\srr\()_user: /* make backtraces match the _kernel variant */
> addi r3,r1,STACK_FRAME_OVERHEAD
> bl interrupt_exit_user_prepare
> ...
> interrupt_return_\srr\()_kernel:
> addi r3,r1,STACK_FRAME_OVERHEAD
> bl interrupt_exit_kernel_prepare
>
> In interrupt_exit_user_restart() and interrupt_exit_kernel_restart(),
> MSR_PR is verified respectively by BUG_ON(!user_mode(regs)) and
> BUG_ON(user_mode(regs)) prior to calling interrupt_exit_user_prepare()
> and interrupt_exit_kernel_prepare().
>
> The verification in interrupt_exit_user_prepare() and
> interrupt_exit_kernel_prepare() are therefore useless and can be removed.
>
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
Probably okay to do now things are ironing out.
Unless we want to make a new define for interrupt handler debug and put
a bunch of these asserts under it. There's quite a lot more here, and
in asm/interrupt.h, etc.
Thanks,
Nick
> ---
> arch/powerpc/kernel/interrupt.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/arch/powerpc/kernel/interrupt.c b/arch/powerpc/kernel/interrupt.c
> index 21bbd615ca41..f26caf911ab5 100644
> --- a/arch/powerpc/kernel/interrupt.c
> +++ b/arch/powerpc/kernel/interrupt.c
> @@ -465,7 +465,6 @@ notrace unsigned long interrupt_exit_user_prepare(struct pt_regs *regs)
>
> if (!IS_ENABLED(CONFIG_BOOKE) && !IS_ENABLED(CONFIG_40x))
> BUG_ON(!(regs->msr & MSR_RI));
> - BUG_ON(!(regs->msr & MSR_PR));
> BUG_ON(arch_irq_disabled_regs(regs));
> CT_WARN_ON(ct_state() == CONTEXT_USER);
>
> @@ -499,7 +498,6 @@ notrace unsigned long interrupt_exit_kernel_prepare(struct pt_regs *regs)
> if (!IS_ENABLED(CONFIG_BOOKE) && !IS_ENABLED(CONFIG_40x) &&
> unlikely(!(regs->msr & MSR_RI)))
> unrecoverable_exception(regs);
> - BUG_ON(regs->msr & MSR_PR);
> /*
> * CT_WARN_ON comes here via program_check_exception,
> * so avoid recursion.
> --
> 2.25.0
>
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] powerpc: Remove MSR_PR check in interrupt_exit_{user/kernel}_prepare()
2021-08-04 8:08 ` Nicholas Piggin
@ 2021-08-04 8:37 ` Christophe Leroy
-1 siblings, 0 replies; 8+ messages in thread
From: Christophe Leroy @ 2021-08-04 8:37 UTC (permalink / raw)
To: Nicholas Piggin, Benjamin Herrenschmidt, Michael Ellerman,
Paul Mackerras
Cc: linux-kernel, linuxppc-dev
Le 04/08/2021 à 10:08, Nicholas Piggin a écrit :
> Excerpts from Christophe Leroy's message of August 4, 2021 3:27 pm:
>> In those hot functions that are called at every interrupt, any saved
>> cycle is worth it.
>>
>> interrupt_exit_user_prepare() and interrupt_exit_kernel_prepare() are
>> called from three places:
>> - From entry_32.S
>> - From interrupt_64.S
>> - From interrupt_exit_user_restart() and interrupt_exit_kernel_restart()
>>
>> In entry_32.S, there are inambiguously called based on MSR_PR:
>>
>> interrupt_return:
>> lwz r4,_MSR(r1)
>> addi r3,r1,STACK_FRAME_OVERHEAD
>> andi. r0,r4,MSR_PR
>> beq .Lkernel_interrupt_return
>> bl interrupt_exit_user_prepare
>> ...
>> .Lkernel_interrupt_return:
>> bl interrupt_exit_kernel_prepare
>>
>> In interrupt_64.S, that's similar:
>>
>> interrupt_return_\srr\():
>> ld r4,_MSR(r1)
>> andi. r0,r4,MSR_PR
>> beq interrupt_return_\srr\()_kernel
>> interrupt_return_\srr\()_user: /* make backtraces match the _kernel variant */
>> addi r3,r1,STACK_FRAME_OVERHEAD
>> bl interrupt_exit_user_prepare
>> ...
>> interrupt_return_\srr\()_kernel:
>> addi r3,r1,STACK_FRAME_OVERHEAD
>> bl interrupt_exit_kernel_prepare
>>
>> In interrupt_exit_user_restart() and interrupt_exit_kernel_restart(),
>> MSR_PR is verified respectively by BUG_ON(!user_mode(regs)) and
>> BUG_ON(user_mode(regs)) prior to calling interrupt_exit_user_prepare()
>> and interrupt_exit_kernel_prepare().
>>
>> The verification in interrupt_exit_user_prepare() and
>> interrupt_exit_kernel_prepare() are therefore useless and can be removed.
>>
>> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
>
> Probably okay to do now things are ironing out.
>
> Unless we want to make a new define for interrupt handler debug and put
> a bunch of these asserts under it. There's quite a lot more here, and
> in asm/interrupt.h, etc.
But that one is so trivial that I'm not sure there is any point in keeping it even as a kind of
additional DEBUG level, unless you want those BUG_ONs because you don't trust the compiler.
Christophe
>
> Thanks,
> Nick
>
>> ---
>> arch/powerpc/kernel/interrupt.c | 2 --
>> 1 file changed, 2 deletions(-)
>>
>> diff --git a/arch/powerpc/kernel/interrupt.c b/arch/powerpc/kernel/interrupt.c
>> index 21bbd615ca41..f26caf911ab5 100644
>> --- a/arch/powerpc/kernel/interrupt.c
>> +++ b/arch/powerpc/kernel/interrupt.c
>> @@ -465,7 +465,6 @@ notrace unsigned long interrupt_exit_user_prepare(struct pt_regs *regs)
>>
>> if (!IS_ENABLED(CONFIG_BOOKE) && !IS_ENABLED(CONFIG_40x))
>> BUG_ON(!(regs->msr & MSR_RI));
>> - BUG_ON(!(regs->msr & MSR_PR));
>> BUG_ON(arch_irq_disabled_regs(regs));
>> CT_WARN_ON(ct_state() == CONTEXT_USER);
>>
>> @@ -499,7 +498,6 @@ notrace unsigned long interrupt_exit_kernel_prepare(struct pt_regs *regs)
>> if (!IS_ENABLED(CONFIG_BOOKE) && !IS_ENABLED(CONFIG_40x) &&
>> unlikely(!(regs->msr & MSR_RI)))
>> unrecoverable_exception(regs);
>> - BUG_ON(regs->msr & MSR_PR);
>> /*
>> * CT_WARN_ON comes here via program_check_exception,
>> * so avoid recursion.
>> --
>> 2.25.0
>>
>>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] powerpc: Remove MSR_PR check in interrupt_exit_{user/kernel}_prepare()
@ 2021-08-04 8:37 ` Christophe Leroy
0 siblings, 0 replies; 8+ messages in thread
From: Christophe Leroy @ 2021-08-04 8:37 UTC (permalink / raw)
To: Nicholas Piggin, Benjamin Herrenschmidt, Michael Ellerman,
Paul Mackerras
Cc: linuxppc-dev, linux-kernel
Le 04/08/2021 à 10:08, Nicholas Piggin a écrit :
> Excerpts from Christophe Leroy's message of August 4, 2021 3:27 pm:
>> In those hot functions that are called at every interrupt, any saved
>> cycle is worth it.
>>
>> interrupt_exit_user_prepare() and interrupt_exit_kernel_prepare() are
>> called from three places:
>> - From entry_32.S
>> - From interrupt_64.S
>> - From interrupt_exit_user_restart() and interrupt_exit_kernel_restart()
>>
>> In entry_32.S, there are inambiguously called based on MSR_PR:
>>
>> interrupt_return:
>> lwz r4,_MSR(r1)
>> addi r3,r1,STACK_FRAME_OVERHEAD
>> andi. r0,r4,MSR_PR
>> beq .Lkernel_interrupt_return
>> bl interrupt_exit_user_prepare
>> ...
>> .Lkernel_interrupt_return:
>> bl interrupt_exit_kernel_prepare
>>
>> In interrupt_64.S, that's similar:
>>
>> interrupt_return_\srr\():
>> ld r4,_MSR(r1)
>> andi. r0,r4,MSR_PR
>> beq interrupt_return_\srr\()_kernel
>> interrupt_return_\srr\()_user: /* make backtraces match the _kernel variant */
>> addi r3,r1,STACK_FRAME_OVERHEAD
>> bl interrupt_exit_user_prepare
>> ...
>> interrupt_return_\srr\()_kernel:
>> addi r3,r1,STACK_FRAME_OVERHEAD
>> bl interrupt_exit_kernel_prepare
>>
>> In interrupt_exit_user_restart() and interrupt_exit_kernel_restart(),
>> MSR_PR is verified respectively by BUG_ON(!user_mode(regs)) and
>> BUG_ON(user_mode(regs)) prior to calling interrupt_exit_user_prepare()
>> and interrupt_exit_kernel_prepare().
>>
>> The verification in interrupt_exit_user_prepare() and
>> interrupt_exit_kernel_prepare() are therefore useless and can be removed.
>>
>> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
>
> Probably okay to do now things are ironing out.
>
> Unless we want to make a new define for interrupt handler debug and put
> a bunch of these asserts under it. There's quite a lot more here, and
> in asm/interrupt.h, etc.
But that one is so trivial that I'm not sure there is any point in keeping it even as a kind of
additional DEBUG level, unless you want those BUG_ONs because you don't trust the compiler.
Christophe
>
> Thanks,
> Nick
>
>> ---
>> arch/powerpc/kernel/interrupt.c | 2 --
>> 1 file changed, 2 deletions(-)
>>
>> diff --git a/arch/powerpc/kernel/interrupt.c b/arch/powerpc/kernel/interrupt.c
>> index 21bbd615ca41..f26caf911ab5 100644
>> --- a/arch/powerpc/kernel/interrupt.c
>> +++ b/arch/powerpc/kernel/interrupt.c
>> @@ -465,7 +465,6 @@ notrace unsigned long interrupt_exit_user_prepare(struct pt_regs *regs)
>>
>> if (!IS_ENABLED(CONFIG_BOOKE) && !IS_ENABLED(CONFIG_40x))
>> BUG_ON(!(regs->msr & MSR_RI));
>> - BUG_ON(!(regs->msr & MSR_PR));
>> BUG_ON(arch_irq_disabled_regs(regs));
>> CT_WARN_ON(ct_state() == CONTEXT_USER);
>>
>> @@ -499,7 +498,6 @@ notrace unsigned long interrupt_exit_kernel_prepare(struct pt_regs *regs)
>> if (!IS_ENABLED(CONFIG_BOOKE) && !IS_ENABLED(CONFIG_40x) &&
>> unlikely(!(regs->msr & MSR_RI)))
>> unrecoverable_exception(regs);
>> - BUG_ON(regs->msr & MSR_PR);
>> /*
>> * CT_WARN_ON comes here via program_check_exception,
>> * so avoid recursion.
>> --
>> 2.25.0
>>
>>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] powerpc: Remove MSR_PR check in interrupt_exit_{user/kernel}_prepare()
2021-08-04 8:37 ` Christophe Leroy
@ 2021-08-04 8:53 ` Nicholas Piggin
-1 siblings, 0 replies; 8+ messages in thread
From: Nicholas Piggin @ 2021-08-04 8:53 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Christophe Leroy, Michael Ellerman,
Paul Mackerras
Cc: linux-kernel, linuxppc-dev
Excerpts from Christophe Leroy's message of August 4, 2021 6:37 pm:
>
>
> Le 04/08/2021 à 10:08, Nicholas Piggin a écrit :
>> Excerpts from Christophe Leroy's message of August 4, 2021 3:27 pm:
>>> In those hot functions that are called at every interrupt, any saved
>>> cycle is worth it.
>>>
>>> interrupt_exit_user_prepare() and interrupt_exit_kernel_prepare() are
>>> called from three places:
>>> - From entry_32.S
>>> - From interrupt_64.S
>>> - From interrupt_exit_user_restart() and interrupt_exit_kernel_restart()
>>>
>>> In entry_32.S, there are inambiguously called based on MSR_PR:
>>>
>>> interrupt_return:
>>> lwz r4,_MSR(r1)
>>> addi r3,r1,STACK_FRAME_OVERHEAD
>>> andi. r0,r4,MSR_PR
>>> beq .Lkernel_interrupt_return
>>> bl interrupt_exit_user_prepare
>>> ...
>>> .Lkernel_interrupt_return:
>>> bl interrupt_exit_kernel_prepare
>>>
>>> In interrupt_64.S, that's similar:
>>>
>>> interrupt_return_\srr\():
>>> ld r4,_MSR(r1)
>>> andi. r0,r4,MSR_PR
>>> beq interrupt_return_\srr\()_kernel
>>> interrupt_return_\srr\()_user: /* make backtraces match the _kernel variant */
>>> addi r3,r1,STACK_FRAME_OVERHEAD
>>> bl interrupt_exit_user_prepare
>>> ...
>>> interrupt_return_\srr\()_kernel:
>>> addi r3,r1,STACK_FRAME_OVERHEAD
>>> bl interrupt_exit_kernel_prepare
>>>
>>> In interrupt_exit_user_restart() and interrupt_exit_kernel_restart(),
>>> MSR_PR is verified respectively by BUG_ON(!user_mode(regs)) and
>>> BUG_ON(user_mode(regs)) prior to calling interrupt_exit_user_prepare()
>>> and interrupt_exit_kernel_prepare().
>>>
>>> The verification in interrupt_exit_user_prepare() and
>>> interrupt_exit_kernel_prepare() are therefore useless and can be removed.
>>>
>>> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
>>
>> Probably okay to do now things are ironing out.
>>
>> Unless we want to make a new define for interrupt handler debug and put
>> a bunch of these asserts under it. There's quite a lot more here, and
>> in asm/interrupt.h, etc.
>
> But that one is so trivial that I'm not sure there is any point in keeping it even as a kind of
> additional DEBUG level, unless you want those BUG_ONs because you don't trust the compiler.
Fair point.
Acked-by: Nicholas Piggin <npiggin@gmail.com>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] powerpc: Remove MSR_PR check in interrupt_exit_{user/kernel}_prepare()
@ 2021-08-04 8:53 ` Nicholas Piggin
0 siblings, 0 replies; 8+ messages in thread
From: Nicholas Piggin @ 2021-08-04 8:53 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Christophe Leroy, Michael Ellerman,
Paul Mackerras
Cc: linuxppc-dev, linux-kernel
Excerpts from Christophe Leroy's message of August 4, 2021 6:37 pm:
>
>
> Le 04/08/2021 à 10:08, Nicholas Piggin a écrit :
>> Excerpts from Christophe Leroy's message of August 4, 2021 3:27 pm:
>>> In those hot functions that are called at every interrupt, any saved
>>> cycle is worth it.
>>>
>>> interrupt_exit_user_prepare() and interrupt_exit_kernel_prepare() are
>>> called from three places:
>>> - From entry_32.S
>>> - From interrupt_64.S
>>> - From interrupt_exit_user_restart() and interrupt_exit_kernel_restart()
>>>
>>> In entry_32.S, there are inambiguously called based on MSR_PR:
>>>
>>> interrupt_return:
>>> lwz r4,_MSR(r1)
>>> addi r3,r1,STACK_FRAME_OVERHEAD
>>> andi. r0,r4,MSR_PR
>>> beq .Lkernel_interrupt_return
>>> bl interrupt_exit_user_prepare
>>> ...
>>> .Lkernel_interrupt_return:
>>> bl interrupt_exit_kernel_prepare
>>>
>>> In interrupt_64.S, that's similar:
>>>
>>> interrupt_return_\srr\():
>>> ld r4,_MSR(r1)
>>> andi. r0,r4,MSR_PR
>>> beq interrupt_return_\srr\()_kernel
>>> interrupt_return_\srr\()_user: /* make backtraces match the _kernel variant */
>>> addi r3,r1,STACK_FRAME_OVERHEAD
>>> bl interrupt_exit_user_prepare
>>> ...
>>> interrupt_return_\srr\()_kernel:
>>> addi r3,r1,STACK_FRAME_OVERHEAD
>>> bl interrupt_exit_kernel_prepare
>>>
>>> In interrupt_exit_user_restart() and interrupt_exit_kernel_restart(),
>>> MSR_PR is verified respectively by BUG_ON(!user_mode(regs)) and
>>> BUG_ON(user_mode(regs)) prior to calling interrupt_exit_user_prepare()
>>> and interrupt_exit_kernel_prepare().
>>>
>>> The verification in interrupt_exit_user_prepare() and
>>> interrupt_exit_kernel_prepare() are therefore useless and can be removed.
>>>
>>> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
>>
>> Probably okay to do now things are ironing out.
>>
>> Unless we want to make a new define for interrupt handler debug and put
>> a bunch of these asserts under it. There's quite a lot more here, and
>> in asm/interrupt.h, etc.
>
> But that one is so trivial that I'm not sure there is any point in keeping it even as a kind of
> additional DEBUG level, unless you want those BUG_ONs because you don't trust the compiler.
Fair point.
Acked-by: Nicholas Piggin <npiggin@gmail.com>
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-08-04 8:54 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-08-04 5:27 [PATCH] powerpc: Remove MSR_PR check in interrupt_exit_{user/kernel}_prepare() Christophe Leroy
2021-08-04 5:27 ` Christophe Leroy
2021-08-04 8:08 ` Nicholas Piggin
2021-08-04 8:08 ` Nicholas Piggin
2021-08-04 8:37 ` Christophe Leroy
2021-08-04 8:37 ` Christophe Leroy
2021-08-04 8:53 ` Nicholas Piggin
2021-08-04 8:53 ` Nicholas Piggin
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.