diff mbox series

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

Message ID 20211015154624.922960-5-npiggin@gmail.com (mailing list archive)
State Superseded
Headers show
Series powerpc: Make hash MMU code build configurable | expand
Related show

Commit Message

Nicholas Piggin Oct. 15, 2021, 3:46 p.m. UTC 
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(-)

Comments

Christophe Leroy Oct. 18, 2021, 5:09 p.m. UTC | #1 
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.
Nicholas Piggin Oct. 20, 2021, 5:07 a.m. UTC | #2 
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
diff mbox series

Patch

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