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

[Christophe Leroy]

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.