Re: [PATCH v5 18/22] powerpc/syscall: Remove FULL_REGS verification in system_call_exception

2021-02-09 Thread Nicholas Piggin
Excerpts from Christophe Leroy's message of February 10, 2021 12:31 am:
> 
> 
> Le 09/02/2021 à 03:02, Nicholas Piggin a écrit :
>> Excerpts from Christophe Leroy's message of February 9, 2021 1:10 am:
>>> For book3s/64, FULL_REGS() is 'true' at all time, so the test voids.
>>> For others, non volatile registers are saved inconditionally.
>>>
>>> So the verification is pointless.
>>>
>>> Should one fail to do it, it would anyway be caught by the
>>> CHECK_FULL_REGS() in copy_thread() as we have removed the
>>> special versions ppc_fork() and friends.
>>>
>>> null_syscall benchmark reduction 4 cycles (332 => 328 cycles)
>> 
>> I wonder if we rather make a CONFIG option for a bunch of these simpler
>> debug checks here (and also in interrupt exit, wrappers, etc) rather
>> than remove them entirely.
> 
> We can drop this patch if you prefer. Anyway, like book3s/64, once ppc32 also 
> do interrupt 
> entry/exit in C, FULL_REGS() will already return true.

Sure let's do that.

Thanks,
Nick



Re: [PATCH v5 18/22] powerpc/syscall: Remove FULL_REGS verification in system_call_exception

2021-02-09 Thread Christophe Leroy




Le 09/02/2021 à 03:02, Nicholas Piggin a écrit :

Excerpts from Christophe Leroy's message of February 9, 2021 1:10 am:

For book3s/64, FULL_REGS() is 'true' at all time, so the test voids.
For others, non volatile registers are saved inconditionally.

So the verification is pointless.

Should one fail to do it, it would anyway be caught by the
CHECK_FULL_REGS() in copy_thread() as we have removed the
special versions ppc_fork() and friends.

null_syscall benchmark reduction 4 cycles (332 => 328 cycles)


I wonder if we rather make a CONFIG option for a bunch of these simpler
debug checks here (and also in interrupt exit, wrappers, etc) rather
than remove them entirely.


We can drop this patch if you prefer. Anyway, like book3s/64, once ppc32 also do interrupt 
entry/exit in C, FULL_REGS() will already return true.


Christophe




Thanks,
Nick



Signed-off-by: Christophe Leroy 
---
  arch/powerpc/kernel/interrupt.c | 1 -
  1 file changed, 1 deletion(-)

diff --git a/arch/powerpc/kernel/interrupt.c b/arch/powerpc/kernel/interrupt.c
index 8fafca727b8b..55e1aa18cdb9 100644
--- a/arch/powerpc/kernel/interrupt.c
+++ b/arch/powerpc/kernel/interrupt.c
@@ -42,7 +42,6 @@ notrace long system_call_exception(long r3, long r4, long r5,
if (!IS_ENABLED(CONFIG_BOOKE) && !IS_ENABLED(CONFIG_40x))
BUG_ON(!(regs->msr & MSR_RI));
BUG_ON(!(regs->msr & MSR_PR));
-   BUG_ON(!FULL_REGS(regs));
BUG_ON(arch_irq_disabled_regs(regs));
  
  #ifdef CONFIG_PPC_PKEY

--
2.25.0




Re: [PATCH v5 18/22] powerpc/syscall: Remove FULL_REGS verification in system_call_exception

2021-02-08 Thread Nicholas Piggin
Excerpts from Christophe Leroy's message of February 9, 2021 1:10 am:
> For book3s/64, FULL_REGS() is 'true' at all time, so the test voids.
> For others, non volatile registers are saved inconditionally.
> 
> So the verification is pointless.
> 
> Should one fail to do it, it would anyway be caught by the
> CHECK_FULL_REGS() in copy_thread() as we have removed the
> special versions ppc_fork() and friends.
> 
> null_syscall benchmark reduction 4 cycles (332 => 328 cycles)

I wonder if we rather make a CONFIG option for a bunch of these simpler
debug checks here (and also in interrupt exit, wrappers, etc) rather
than remove them entirely.

Thanks,
Nick

> 
> Signed-off-by: Christophe Leroy 
> ---
>  arch/powerpc/kernel/interrupt.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/arch/powerpc/kernel/interrupt.c b/arch/powerpc/kernel/interrupt.c
> index 8fafca727b8b..55e1aa18cdb9 100644
> --- a/arch/powerpc/kernel/interrupt.c
> +++ b/arch/powerpc/kernel/interrupt.c
> @@ -42,7 +42,6 @@ notrace long system_call_exception(long r3, long r4, long 
> r5,
>   if (!IS_ENABLED(CONFIG_BOOKE) && !IS_ENABLED(CONFIG_40x))
>   BUG_ON(!(regs->msr & MSR_RI));
>   BUG_ON(!(regs->msr & MSR_PR));
> - BUG_ON(!FULL_REGS(regs));
>   BUG_ON(arch_irq_disabled_regs(regs));
>  
>  #ifdef CONFIG_PPC_PKEY
> -- 
> 2.25.0
> 
>