Re: [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.