Re: [PATCH 03/18] powerpc: bad_page_fault, do_break get registers from regs
Excerpts from Nicholas Piggin's message of November 11, 2020 2:46 pm: > Excerpts from Christophe Leroy's message of November 10, 2020 9:19 pm: >> >> >> Le 10/11/2020 à 09:34, Nicholas Piggin a écrit : >>> Excerpts from Christophe Leroy's message of November 6, 2020 6:14 pm: Le 05/11/2020 à 15:34, Nicholas Piggin a écrit : > This also moves the 32s DABR match to C. Is there a real benefit doing this ? >>> >>> Oh I missed doing it, but yes I think bad_page_fault and do_break should >>> probably be implemented with the DEFINE_INTERRUT_HANDLER wrappers. >>> >> >> Yes, anyway, do we need to do that change ? Can't the dispatch between >> do_break() and page fault >> handling remain in handle_page_fault() ? What's the benefit of going into >> do_page_fault() and coming >> back ? > > You might be right, I'll take another look at it. For 32-bit, we need to come back to save NV GPRs. Certainly the 64s code stays in do_page_fault because it always saves them. Now I don't think that's the nicest thing to go in and out of the interrupt wrappers twice in these cases, but for a first pass I think it's okay. Either we could add another type of error-case wrapper that does some adjustment if it becomes necessary, or we find a nice way to save NVGPRs from C code. If we could somehow parse unwind data to find where the NVGPRs are saved by the compiler and generate a little code stub to load them out, would be the ultimate :) Maybe one day... Thanks, Nick
Re: [PATCH 03/18] powerpc: bad_page_fault, do_break get registers from regs
Excerpts from Christophe Leroy's message of November 10, 2020 9:19 pm: > > > Le 10/11/2020 à 09:34, Nicholas Piggin a écrit : >> Excerpts from Christophe Leroy's message of November 6, 2020 6:14 pm: >>> >>> >>> Le 05/11/2020 à 15:34, Nicholas Piggin a écrit : This also moves the 32s DABR match to C. >>> >>> Is there a real benefit doing this ? >> >> Oh I missed doing it, but yes I think bad_page_fault and do_break should >> probably be implemented with the DEFINE_INTERRUT_HANDLER wrappers. >> > > Yes, anyway, do we need to do that change ? Can't the dispatch between > do_break() and page fault > handling remain in handle_page_fault() ? What's the benefit of going into > do_page_fault() and coming > back ? You might be right, I'll take another look at it. Thanks, Nick
Re: [PATCH 03/18] powerpc: bad_page_fault, do_break get registers from regs
Le 10/11/2020 à 09:34, Nicholas Piggin a écrit : Excerpts from Christophe Leroy's message of November 6, 2020 6:14 pm: Le 05/11/2020 à 15:34, Nicholas Piggin a écrit : This also moves the 32s DABR match to C. Is there a real benefit doing this ? Oh I missed doing it, but yes I think bad_page_fault and do_break should probably be implemented with the DEFINE_INTERRUT_HANDLER wrappers. Yes, anyway, do we need to do that change ? Can't the dispatch between do_break() and page fault handling remain in handle_page_fault() ? What's the benefit of going into do_page_fault() and coming back ? Christophe
Re: [PATCH 03/18] powerpc: bad_page_fault, do_break get registers from regs
Excerpts from Christophe Leroy's message of November 6, 2020 6:14 pm: > > > Le 05/11/2020 à 15:34, Nicholas Piggin a écrit : >> This also moves the 32s DABR match to C. > > Is there a real benefit doing this ? Oh I missed doing it, but yes I think bad_page_fault and do_break should probably be implemented with the DEFINE_INTERRUT_HANDLER wrappers. Thanks, Nick
Re: [PATCH 03/18] powerpc: bad_page_fault, do_break get registers from regs
Le 05/11/2020 à 15:34, Nicholas Piggin a écrit : This also moves the 32s DABR match to C. Is there a real benefit doing this ? Similar to the previous patch this makes interrupt handler function types more regular so they can be wrapped with the next patch. bad_page_fault and do_break are not performance critical. Signed-off-by: Nicholas Piggin --- arch/powerpc/include/asm/bug.h | 2 +- arch/powerpc/include/asm/debug.h | 3 +-- arch/powerpc/kernel/entry_32.S | 14 -- arch/powerpc/kernel/exceptions-64e.S | 3 +-- arch/powerpc/kernel/exceptions-64s.S | 3 +-- arch/powerpc/kernel/head_8xx.S | 5 ++--- arch/powerpc/kernel/process.c | 7 +++ arch/powerpc/kernel/traps.c| 2 +- arch/powerpc/mm/book3s64/hash_utils.c | 4 ++-- arch/powerpc/mm/book3s64/slb.c | 2 +- arch/powerpc/mm/fault.c| 14 +++--- arch/powerpc/platforms/8xx/machine_check.c | 2 +- 12 files changed, 25 insertions(+), 36 deletions(-) diff --git a/arch/powerpc/include/asm/bug.h b/arch/powerpc/include/asm/bug.h index 2fa0cf6c6011..4af6c3835eb2 100644 --- a/arch/powerpc/include/asm/bug.h +++ b/arch/powerpc/include/asm/bug.h @@ -113,7 +113,7 @@ struct pt_regs; extern long do_page_fault(struct pt_regs *); extern long hash__do_page_fault(struct pt_regs *); -extern void bad_page_fault(struct pt_regs *, unsigned long, int); +extern void bad_page_fault(struct pt_regs *, int); pointless extern Christophe extern void _exception(int, struct pt_regs *, int, unsigned long); extern void _exception_pkey(struct pt_regs *, unsigned long, int); extern void die(const char *, struct pt_regs *, long); diff --git a/arch/powerpc/include/asm/debug.h b/arch/powerpc/include/asm/debug.h index ec57daf87f40..0550eceab3ca 100644 --- a/arch/powerpc/include/asm/debug.h +++ b/arch/powerpc/include/asm/debug.h @@ -52,8 +52,7 @@ extern void do_send_trap(struct pt_regs *regs, unsigned long address, unsigned long error_code, int brkpt); #else -extern void do_break(struct pt_regs *regs, unsigned long address, -unsigned long error_code); +void do_break(struct pt_regs *regs); #endif #endif /* _ASM_POWERPC_DEBUG_H */ diff --git a/arch/powerpc/kernel/entry_32.S b/arch/powerpc/kernel/entry_32.S index 8cdc8bcde703..eb97df234a0c 100644 --- a/arch/powerpc/kernel/entry_32.S +++ b/arch/powerpc/kernel/entry_32.S @@ -657,10 +657,6 @@ ppc_swapcontext: .globl handle_page_fault handle_page_fault: addir3,r1,STACK_FRAME_OVERHEAD -#ifdef CONFIG_PPC_BOOK3S_32 - andis. r0,r5,DSISR_DABRMATCH@h - bne-handle_dabr_fault -#endif bl do_page_fault cmpwi r3,0 beq+ret_from_except @@ -668,19 +664,17 @@ handle_page_fault: lwz r0,_TRAP(r1) clrrwi r0,r0,1 stw r0,_TRAP(r1) - mr r5,r3 + mr r4,r3 /* err arg for bad_page_fault */ addir3,r1,STACK_FRAME_OVERHEAD - lwz r4,_DAR(r1) +#ifdef CONFIG_PPC_BOOK3S_32 + blt handle_dabr_fault +#endif bl bad_page_fault b ret_from_except_full #ifdef CONFIG_PPC_BOOK3S_32 /* We have a data breakpoint exception - handle it */ handle_dabr_fault: - SAVE_NVGPRS(r1) - lwz r0,_TRAP(r1) - clrrwi r0,r0,1 - stw r0,_TRAP(r1) bl do_break b ret_from_except_full #endif diff --git a/arch/powerpc/kernel/exceptions-64e.S b/arch/powerpc/kernel/exceptions-64e.S index 25fa7d5a643c..dc728bb1c89a 100644 --- a/arch/powerpc/kernel/exceptions-64e.S +++ b/arch/powerpc/kernel/exceptions-64e.S @@ -1018,9 +1018,8 @@ storage_fault_common: bne-1f b ret_from_except_lite 1:bl save_nvgprs - mr r5,r3 + mr r4,r3 addir3,r1,STACK_FRAME_OVERHEAD - ld r4,_DAR(r1) bl bad_page_fault b ret_from_except diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S index 1f34cfd1887c..e6558c4d3f81 100644 --- a/arch/powerpc/kernel/exceptions-64s.S +++ b/arch/powerpc/kernel/exceptions-64s.S @@ -2135,8 +2135,7 @@ EXC_COMMON_BEGIN(h_data_storage_common) GEN_COMMON h_data_storage addir3,r1,STACK_FRAME_OVERHEAD BEGIN_MMU_FTR_SECTION - ld r4,_DAR(r1) - li r5,SIGSEGV + li r4,SIGSEGV bl bad_page_fault MMU_FTR_SECTION_ELSE bl unknown_exception diff --git a/arch/powerpc/kernel/head_8xx.S b/arch/powerpc/kernel/head_8xx.S index 0cd95b633e2b..13eda7154695 100644 --- a/arch/powerpc/kernel/head_8xx.S +++ b/arch/powerpc/kernel/head_8xx.S @@ -408,10 +408,9 @@ do_databreakpoint: addir3,r1,STACK_FRAME_OVERHEAD mfspr r4,SPRN_BAR stw r4,_DAR(r11) -#ifdef CONFIG_VMAP_STACK
Re: [PATCH 03/18] powerpc: bad_page_fault, do_break get registers from regs
Hi Nicholas,
I love your patch! Yet something to improve:
[auto build test ERROR on powerpc/next]
[also build test ERROR on v5.10-rc2 next-20201105]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url:
https://github.com/0day-ci/linux/commits/Nicholas-Piggin/powerpc-interrupt-wrappers/20201105-231909
base: https://git.kernel.org/pub/scm/linux/kernel/git/powerpc/linux.git next
config: powerpc-allnoconfig (attached as .config)
compiler: powerpc-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
#
https://github.com/0day-ci/linux/commit/4232a616cd2a8f7ef6b3f19cd656690dc5ec4c9e
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review
Nicholas-Piggin/powerpc-interrupt-wrappers/20201105-231909
git checkout 4232a616cd2a8f7ef6b3f19cd656690dc5ec4c9e
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross
ARCH=powerpc
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot
All errors (new ones prefixed by >>):
In file included from include/asm-generic/bug.h:5,
from arch/powerpc/include/asm/bug.h:109,
from include/linux/bug.h:5,
from include/linux/signal.h:5,
from arch/powerpc/mm/fault.c:14:
arch/powerpc/mm/fault.c: In function '__do_page_fault':
>> arch/powerpc/mm/fault.c:378:43: error: suggest parentheses around arithmetic
>> in operand of '|' [-Werror=parentheses]
378 | #define page_fault_is_bad(__err) ((__err) & DSISR_BAD_FAULT_32S |
DSISR_DABRMATCH)
include/linux/compiler.h:78:42: note: in definition of macro 'unlikely'
78 | # define unlikely(x) __builtin_expect(!!(x), 0)
| ^
arch/powerpc/mm/fault.c:410:15: note: in expansion of macro
'page_fault_is_bad'
410 | if (unlikely(page_fault_is_bad(error_code))) {
| ^
cc1: all warnings being treated as errors
vim +378 arch/powerpc/mm/fault.c
361
362 /*
363 * Define the correct "is_write" bit in error_code based
364 * on the processor family
365 */
366 #if (defined(CONFIG_4xx) || defined(CONFIG_BOOKE))
367 #define page_fault_is_write(__err) ((__err) & ESR_DST)
368 #define page_fault_is_bad(__err)(0)
369 #else
370 #define page_fault_is_write(__err) ((__err) & DSISR_ISSTORE)
371 #if defined(CONFIG_PPC_8xx)
372 #define page_fault_is_bad(__err)((__err) & DSISR_NOEXEC_OR_G)
373 #elif defined(CONFIG_PPC_BOOK3S_64)
374 #define page_fault_is_bad(__err)((__err) & (DSISR_BAD_FAULT_64S
| DSISR_DABRMATCH))
375 #elif defined(CONFIG_PPC_BOOK3E_64)
376 #define page_fault_is_bad(__err)((__err) & DSISR_BAD_FAULT_64S)
377 #else
> 378 #define page_fault_is_bad(__err)((__err) & DSISR_BAD_FAULT_32S
> | DSISR_DABRMATCH)
379 #endif
380 #endif
381
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
.config.gz
Description: application/gzip
[PATCH 03/18] powerpc: bad_page_fault, do_break get registers from regs
This also moves the 32s DABR match to C. Similar to the previous patch this makes interrupt handler function types more regular so they can be wrapped with the next patch. bad_page_fault and do_break are not performance critical. Signed-off-by: Nicholas Piggin --- arch/powerpc/include/asm/bug.h | 2 +- arch/powerpc/include/asm/debug.h | 3 +-- arch/powerpc/kernel/entry_32.S | 14 -- arch/powerpc/kernel/exceptions-64e.S | 3 +-- arch/powerpc/kernel/exceptions-64s.S | 3 +-- arch/powerpc/kernel/head_8xx.S | 5 ++--- arch/powerpc/kernel/process.c | 7 +++ arch/powerpc/kernel/traps.c| 2 +- arch/powerpc/mm/book3s64/hash_utils.c | 4 ++-- arch/powerpc/mm/book3s64/slb.c | 2 +- arch/powerpc/mm/fault.c| 14 +++--- arch/powerpc/platforms/8xx/machine_check.c | 2 +- 12 files changed, 25 insertions(+), 36 deletions(-) diff --git a/arch/powerpc/include/asm/bug.h b/arch/powerpc/include/asm/bug.h index 2fa0cf6c6011..4af6c3835eb2 100644 --- a/arch/powerpc/include/asm/bug.h +++ b/arch/powerpc/include/asm/bug.h @@ -113,7 +113,7 @@ struct pt_regs; extern long do_page_fault(struct pt_regs *); extern long hash__do_page_fault(struct pt_regs *); -extern void bad_page_fault(struct pt_regs *, unsigned long, int); +extern void bad_page_fault(struct pt_regs *, int); extern void _exception(int, struct pt_regs *, int, unsigned long); extern void _exception_pkey(struct pt_regs *, unsigned long, int); extern void die(const char *, struct pt_regs *, long); diff --git a/arch/powerpc/include/asm/debug.h b/arch/powerpc/include/asm/debug.h index ec57daf87f40..0550eceab3ca 100644 --- a/arch/powerpc/include/asm/debug.h +++ b/arch/powerpc/include/asm/debug.h @@ -52,8 +52,7 @@ extern void do_send_trap(struct pt_regs *regs, unsigned long address, unsigned long error_code, int brkpt); #else -extern void do_break(struct pt_regs *regs, unsigned long address, -unsigned long error_code); +void do_break(struct pt_regs *regs); #endif #endif /* _ASM_POWERPC_DEBUG_H */ diff --git a/arch/powerpc/kernel/entry_32.S b/arch/powerpc/kernel/entry_32.S index 8cdc8bcde703..eb97df234a0c 100644 --- a/arch/powerpc/kernel/entry_32.S +++ b/arch/powerpc/kernel/entry_32.S @@ -657,10 +657,6 @@ ppc_swapcontext: .globl handle_page_fault handle_page_fault: addir3,r1,STACK_FRAME_OVERHEAD -#ifdef CONFIG_PPC_BOOK3S_32 - andis. r0,r5,DSISR_DABRMATCH@h - bne-handle_dabr_fault -#endif bl do_page_fault cmpwi r3,0 beq+ret_from_except @@ -668,19 +664,17 @@ handle_page_fault: lwz r0,_TRAP(r1) clrrwi r0,r0,1 stw r0,_TRAP(r1) - mr r5,r3 + mr r4,r3 /* err arg for bad_page_fault */ addir3,r1,STACK_FRAME_OVERHEAD - lwz r4,_DAR(r1) +#ifdef CONFIG_PPC_BOOK3S_32 + blt handle_dabr_fault +#endif bl bad_page_fault b ret_from_except_full #ifdef CONFIG_PPC_BOOK3S_32 /* We have a data breakpoint exception - handle it */ handle_dabr_fault: - SAVE_NVGPRS(r1) - lwz r0,_TRAP(r1) - clrrwi r0,r0,1 - stw r0,_TRAP(r1) bl do_break b ret_from_except_full #endif diff --git a/arch/powerpc/kernel/exceptions-64e.S b/arch/powerpc/kernel/exceptions-64e.S index 25fa7d5a643c..dc728bb1c89a 100644 --- a/arch/powerpc/kernel/exceptions-64e.S +++ b/arch/powerpc/kernel/exceptions-64e.S @@ -1018,9 +1018,8 @@ storage_fault_common: bne-1f b ret_from_except_lite 1: bl save_nvgprs - mr r5,r3 + mr r4,r3 addir3,r1,STACK_FRAME_OVERHEAD - ld r4,_DAR(r1) bl bad_page_fault b ret_from_except diff --git a/arch/powerpc/kernel/exceptions-64s.S b/arch/powerpc/kernel/exceptions-64s.S index 1f34cfd1887c..e6558c4d3f81 100644 --- a/arch/powerpc/kernel/exceptions-64s.S +++ b/arch/powerpc/kernel/exceptions-64s.S @@ -2135,8 +2135,7 @@ EXC_COMMON_BEGIN(h_data_storage_common) GEN_COMMON h_data_storage addir3,r1,STACK_FRAME_OVERHEAD BEGIN_MMU_FTR_SECTION - ld r4,_DAR(r1) - li r5,SIGSEGV + li r4,SIGSEGV bl bad_page_fault MMU_FTR_SECTION_ELSE bl unknown_exception diff --git a/arch/powerpc/kernel/head_8xx.S b/arch/powerpc/kernel/head_8xx.S index 0cd95b633e2b..13eda7154695 100644 --- a/arch/powerpc/kernel/head_8xx.S +++ b/arch/powerpc/kernel/head_8xx.S @@ -408,10 +408,9 @@ do_databreakpoint: addir3,r1,STACK_FRAME_OVERHEAD mfspr r4,SPRN_BAR stw r4,_DAR(r11) -#ifdef CONFIG_VMAP_STACK - lwz r5,_DSISR(r11) -#else +#ifndef CONFIG_VMAP_STACK mfspr r5,SPRN_DSISR + stw r5,_DSISR(r11) #endif EXC_XFER_STD(0x1c00,
