Re: [PATCH v6 05/39] powerpc: remove arguments from fault handler functions

From: Nicholas Piggin <npiggin@gmail.com>
To: Christophe Leroy <christophe.leroy@csgroup.eu>,
	linuxppc-dev@lists.ozlabs.org
Subject: Re: [PATCH v6 05/39] powerpc: remove arguments from fault handler functions
Date: Thu, 28 Jan 2021 10:05:33 +1000	[thread overview]
Message-ID: <1611791489.gto0xh54hf.astroid@bobo.none> (raw)
In-Reply-To: <86bdad6b-2591-6992-b739-caa0874ca558@csgroup.eu>

Excerpts from Christophe Leroy's message of January 27, 2021 4:38 pm:
> 
> 
> Le 15/01/2021 à 17:49, Nicholas Piggin a écrit :
>> Make mm fault handlers all just take the pt_regs * argument and load
>> DAR/DSISR from that. Make those that return a value return long.
>> 
>> This is done to make the function signatures match other handlers, which
>> will help with a future patch to add wrappers. Explicit arguments could
>> be added for performance but that would require more wrapper macro
>> variants.
>> 
>> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
>> ---
>>   arch/powerpc/include/asm/asm-prototypes.h     |  4 ++--
>>   arch/powerpc/include/asm/book3s/64/mmu-hash.h |  2 +-
>>   arch/powerpc/include/asm/bug.h                |  2 +-
>>   arch/powerpc/kernel/entry_32.S                |  7 +------
>>   arch/powerpc/kernel/exceptions-64e.S          |  2 --
>>   arch/powerpc/kernel/exceptions-64s.S          | 17 ++++-------------
>>   arch/powerpc/kernel/head_40x.S                | 10 +++++-----
>>   arch/powerpc/kernel/head_8xx.S                |  6 +++---
>>   arch/powerpc/kernel/head_book3s_32.S          |  5 ++---
>>   arch/powerpc/kernel/head_booke.h              |  4 +---
>>   arch/powerpc/mm/book3s64/hash_utils.c         |  8 +++++---
>>   arch/powerpc/mm/book3s64/slb.c                | 11 +++++++----
>>   arch/powerpc/mm/fault.c                       |  5 ++---
>>   13 files changed, 34 insertions(+), 49 deletions(-)
>> 
> 
>> diff --git a/arch/powerpc/kernel/entry_32.S b/arch/powerpc/kernel/entry_32.S
>> index 238eacfda7b0..d6ea3f2d6cc0 100644
>> --- a/arch/powerpc/kernel/entry_32.S
>> +++ b/arch/powerpc/kernel/entry_32.S
>> @@ -276,8 +276,7 @@ reenable_mmu:
>>   	 * We save a bunch of GPRs,
>>   	 * r3 can be different from GPR3(r1) at this point, r9 and r11
>>   	 * contains the old MSR and handler address respectively,
>> -	 * r4 & r5 can contain page fault arguments that need to be passed
>> -	 * along as well. r0, r6-r8, r12, CCR, CTR, XER etc... are left
>> +	 * r0, r4-r8, r12, CCR, CTR, XER etc... are left
>>   	 * clobbered as they aren't useful past this point.
>>   	 */
>>   
>> @@ -285,15 +284,11 @@ reenable_mmu:
>>   	stw	r9,8(r1)
>>   	stw	r11,12(r1)
>>   	stw	r3,16(r1)
> 
> As all functions only take 'regs' as input parameter, maybe we can avoid saving 'r3' by 
> recalculating it from r1 after the call with 'addi	r3,r1,STACK_FRAME_OVERHEAD' ?

It seems like it. All functions have regs as first parameter already 
don't they? So this change could be done before this patch as well.

> 
>> -	stw	r4,20(r1)
>> -	stw	r5,24(r1)
> 
> Patch 6 needs to go before this change. Probably the easiest would be to apply patch 6 before patch 
> 5. Or this change needs to go after.

Hmm okay thanks for finding that.

Thanks,
Nick