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
