[PATCH v1 04/11] powerpc/64s: Move and rename do_bad_slb_fault as it is not hash specific

[Nicholas Piggin]

Excerpts from Christophe Leroy's message of October 19, 2021 3:09 am:
> 
> 
> Le 15/10/2021 à 17:46, Nicholas Piggin a écrit :
>> slb.c is hash-specific SLB management, but do_bad_slb_fault deals with
>> segment interrupts that occur with radix MMU as well.
>> ---
>>   arch/powerpc/include/asm/interrupt.h |  2 +-
>>   arch/powerpc/kernel/exceptions-64s.S |  4 ++--
>>   arch/powerpc/mm/book3s64/slb.c       | 16 ----------------
>>   arch/powerpc/mm/fault.c              | 17 +++++++++++++++++
>>   4 files changed, 20 insertions(+), 19 deletions(-)
>> 
>> diff --git a/arch/powerpc/include/asm/interrupt.h b/arch/powerpc/include/asm/interrupt.h
>> index a1d238255f07..3487aab12229 100644
>> --- a/arch/powerpc/include/asm/interrupt.h
>> +++ b/arch/powerpc/include/asm/interrupt.h
>> @@ -564,7 +564,7 @@ DECLARE_INTERRUPT_HANDLER(kernel_bad_stack);
>>   
>>   /* slb.c */
>>   DECLARE_INTERRUPT_HANDLER_RAW(do_slb_fault);
>> -DECLARE_INTERRUPT_HANDLER(do_bad_slb_fault);
>> +DECLARE_INTERRUPT_HANDLER(do_bad_segment_interrupt);
>>   
>>   /* hash_utils.c */
>>   DECLARE_INTERRUPT_HANDLER_RAW(do_hash_fault);
>> diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S
>> index eaf1f72131a1..046c99e31d01 100644
>> --- a/arch/powerpc/kernel/exceptions-64s.S
>> +++ b/arch/powerpc/kernel/exceptions-64s.S
>> @@ -1430,7 +1430,7 @@ MMU_FTR_SECTION_ELSE
>>   ALT_MMU_FTR_SECTION_END_IFCLR(MMU_FTR_TYPE_RADIX)
>>   	std	r3,RESULT(r1)
>>   	addi	r3,r1,STACK_FRAME_OVERHEAD
>> -	bl	do_bad_slb_fault
>> +	bl	do_bad_segment_interrupt
>>   	b	interrupt_return_srr
>>   
>>   
>> @@ -1510,7 +1510,7 @@ MMU_FTR_SECTION_ELSE
>>   ALT_MMU_FTR_SECTION_END_IFCLR(MMU_FTR_TYPE_RADIX)
>>   	std	r3,RESULT(r1)
>>   	addi	r3,r1,STACK_FRAME_OVERHEAD
>> -	bl	do_bad_slb_fault
>> +	bl	do_bad_segment_interrupt
>>   	b	interrupt_return_srr
>>   
>>   
>> diff --git a/arch/powerpc/mm/book3s64/slb.c b/arch/powerpc/mm/book3s64/slb.c
>> index f0037bcc47a0..31f4cef3adac 100644
>> --- a/arch/powerpc/mm/book3s64/slb.c
>> +++ b/arch/powerpc/mm/book3s64/slb.c
>> @@ -868,19 +868,3 @@ DEFINE_INTERRUPT_HANDLER_RAW(do_slb_fault)
>>   		return err;
>>   	}
>>   }
>> -
>> -DEFINE_INTERRUPT_HANDLER(do_bad_slb_fault)
>> -{
>> -	int err = regs->result;
>> -
>> -	if (err == -EFAULT) {
>> -		if (user_mode(regs))
>> -			_exception(SIGSEGV, regs, SEGV_BNDERR, regs->dar);
>> -		else
>> -			bad_page_fault(regs, SIGSEGV);
>> -	} else if (err == -EINVAL) {
>> -		unrecoverable_exception(regs);
>> -	} else {
>> -		BUG();
>> -	}
>> -}
>> diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c
>> index a8d0ce85d39a..53ddcae0ac9e 100644
>> --- a/arch/powerpc/mm/fault.c
>> +++ b/arch/powerpc/mm/fault.c
>> @@ -35,6 +35,7 @@
>>   #include <linux/kfence.h>
>>   #include <linux/pkeys.h>
>>   
>> +#include <asm/asm-prototypes.h>
>>   #include <asm/firmware.h>
>>   #include <asm/interrupt.h>
>>   #include <asm/page.h>
>> @@ -620,4 +621,20 @@ DEFINE_INTERRUPT_HANDLER(do_bad_page_fault_segv)
>>   {
>>   	bad_page_fault(regs, SIGSEGV);
>>   }
>> +
>> +DEFINE_INTERRUPT_HANDLER(do_bad_segment_interrupt)
>> +{
>> +	int err = regs->result;
>> +
>> +	if (err == -EFAULT) {
>> +		if (user_mode(regs))
>> +			_exception(SIGSEGV, regs, SEGV_BNDERR, regs->dar);
>> +		else
>> +			bad_page_fault(regs, SIGSEGV);
>> +	} else if (err == -EINVAL) {
>> +		unrecoverable_exception(regs);
>> +	} else {
>> +		BUG();
>> +	}
>> +}
>>   #endif
>> 
> 
> You could do something more flat:
> 
> 	if (err == -EINVAL)
> 		unrecoverable_exception(regs);
> 
> 	BUG_ON(err != -EFAULT);
> 
> 	if (user_mode(regs))
> 		_exception(SIGSEGV, regs, SEGV_BNDERR, regs->dar);
> 	else
> 		bad_page_fault(regs, SIGSEGV);
> 
> I know you are just moving existing code but moving code is always an 
> opportunity to clean it without additional churn.
> 

Hmm, moving code I prefer not to make any changes. I don't know if it's 
that big an improvement to make the change here.

Thanks,
Nick