[PATCH 02/18] powerpc: remove arguments from fault handler functions

Nicholas Piggin npiggin at gmail.com
Wed Nov 11 15:45:49 AEDT 2020


Excerpts from Christophe Leroy's message of November 10, 2020 9:15 pm:
> 
> 
> Le 10/11/2020 à 09:29, Nicholas Piggin a écrit :
>> Excerpts from Christophe Leroy's message of November 6, 2020 5:59 pm:
>>>
>>>
>>> Le 05/11/2020 à 15:34, 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 at gmail.com>
>>>> ---
> 
> [...]
> 
>> 
>>>> diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c
>>>> index e65a49f246ef..390a296b16a3 100644
>>>> --- a/arch/powerpc/mm/fault.c
>>>> +++ b/arch/powerpc/mm/fault.c
>>>> @@ -549,11 +549,12 @@ static int __do_page_fault(struct pt_regs *regs, unsigned long address,
>>>>    }
>>>>    NOKPROBE_SYMBOL(__do_page_fault);
>>>>    
>>>> -int do_page_fault(struct pt_regs *regs, unsigned long address,
>>>> -		  unsigned long error_code)
>>>> +long do_page_fault(struct pt_regs *regs)
>>>>    {
>>>>    	enum ctx_state prev_state = exception_enter();
>>>> -	int err;
>>>> +	unsigned long address = regs->dar;
>>>> +	unsigned long error_code = regs->dsisr;
>>>> +	long err;
>>>
>>> By doing something more or less like this (need to be tuned for bookE as well):
>>>
>>> +	int is_exec = TRAP(regs) == 0x400;
>>> +	unsigned long address = is_exec ? regs->ssr0 : regs->dar;
>>> +	unsigned long error_code = is_exec ? (regs->ssr1 & DSISR_SRR1_MATCH_32S) : regs->dsisr;
>> 
>> Ah, I didn't see that you saved these in srr0/1 already. Hmm, not in
>> pt_regs though. thread_struct (VMAP_STACK only)? exception_regs (booke
>> only)? Doesn't seem so easy.
> 
> Oops yes you are right, SRR0/SRR1 are not in pt_regs. And their validity in thread struct is rather 
> short ... So forget my comment.

So, are you happy to go with this for now? I guess things can
later be cleaned up to avoid double saving on cases like VMAP.

Thanks,
Nick


More information about the Linuxppc-dev mailing list