[PATCH v2 03/19] powerpc: bad_page_fault, do_break get registers from regs

Nicholas Piggin npiggin at gmail.com
Mon Nov 16 12:52:05 AEDT 2020


Excerpts from Christophe Leroy's message of November 15, 2020 5:07 am:
> Hi,
> 
> Quoting Nicholas Piggin <npiggin at gmail.com>:
> 
>> This also moves the 32s DABR match to C.
> 
> I'm still not happy with that. What about the following instead ?

This seems quite clean. I'll pull it into the series and see how it looks.

Thanks,
Nick

> 
> diff --git a/arch/powerpc/kernel/entry_32.S b/arch/powerpc/kernel/entry_32.S
> index 8cdc8bcde703..6253c4acb46d 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:
>   	addi	r3,r1,STACK_FRAME_OVERHEAD
> -#ifdef CONFIG_PPC_BOOK3S_32
> -	andis.  r0,r5,DSISR_DABRMATCH at h
> -	bne-    handle_dabr_fault
> -#endif
>   	bl	do_page_fault
>   	cmpwi	r3,0
>   	beq+	ret_from_except
> @@ -674,17 +670,6 @@ handle_page_fault:
>   	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
> -
>   /*
>    * This routine switches between two different tasks.  The process
>    * state of one is saved on its kernel stack.  Then the state
> diff --git a/arch/powerpc/kernel/head_book3s_32.S  
> b/arch/powerpc/kernel/head_book3s_32.S
> index 9381aa867591..5cc71482b35f 100644
> --- a/arch/powerpc/kernel/head_book3s_32.S
> +++ b/arch/powerpc/kernel/head_book3s_32.S
> @@ -684,7 +684,10 @@ handle_page_fault_tramp_1:
>   	lwz	r5, _DSISR(r11)
>   	/* fall through */
>   handle_page_fault_tramp_2:
> +	andis.  r0, r5, DSISR_DABRMATCH at h
> +	bne-	1f
>   	EXC_XFER_LITE(0x300, handle_page_fault)
> +1:	EXC_XFER_STD(0x300, do_break)
> 
>   #ifdef CONFIG_VMAP_STACK
>   .macro save_regs_thread		thread
> ---
> Christophe
> 
>>
>> 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 <npiggin at gmail.com>
>> ---
>>  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 897bad6b6bbb..49162faba33f 100644
>> --- a/arch/powerpc/include/asm/bug.h
>> +++ b/arch/powerpc/include/asm/bug.h
>> @@ -113,7 +113,7 @@
>>  struct pt_regs;
>>  long do_page_fault(struct pt_regs *);
>>  long hash__do_page_fault(struct pt_regs *);
>> -extern void bad_page_fault(struct pt_regs *, unsigned long, int);
>> +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:
>>  	addi	r3,r1,STACK_FRAME_OVERHEAD
>> -#ifdef CONFIG_PPC_BOOK3S_32
>> -	andis.  r0,r5,DSISR_DABRMATCH at 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 */
>>  	addi	r3,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
>>  	addi	r3,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
>>  	addi    r3,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:
>>  	addi	r3,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, do_break)
>>
>> diff --git a/arch/powerpc/kernel/process.c b/arch/powerpc/kernel/process.c
>> index d421a2c7f822..0bdd3ed653df 100644
>> --- a/arch/powerpc/kernel/process.c
>> +++ b/arch/powerpc/kernel/process.c
>> @@ -660,11 +660,10 @@ static void do_break_handler(struct pt_regs *regs)
>>  	}
>>  }
>>
>> -void do_break (struct pt_regs *regs, unsigned long address,
>> -		    unsigned long error_code)
>> +void do_break(struct pt_regs *regs)
>>  {
>>  	current->thread.trap_nr = TRAP_HWBKPT;
>> -	if (notify_die(DIE_DABR_MATCH, "dabr_match", regs, error_code,
>> +	if (notify_die(DIE_DABR_MATCH, "dabr_match", regs, regs->dsisr,
>>  			11, SIGSEGV) == NOTIFY_STOP)
>>  		return;
>>
>> @@ -682,7 +681,7 @@ void do_break (struct pt_regs *regs, unsigned  
>> long address,
>>  		do_break_handler(regs);
>>
>>  	/* Deliver the signal to userspace */
>> -	force_sig_fault(SIGTRAP, TRAP_HWBKPT, (void __user *)address);
>> +	force_sig_fault(SIGTRAP, TRAP_HWBKPT, (void __user *)regs->dar);
>>  }
>>  #endif	/* CONFIG_PPC_ADV_DEBUG_REGS */
>>
>> diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
>> index 5006dcbe1d9f..902fcbd1a778 100644
>> --- a/arch/powerpc/kernel/traps.c
>> +++ b/arch/powerpc/kernel/traps.c
>> @@ -1641,7 +1641,7 @@ void alignment_exception(struct pt_regs *regs)
>>  	if (user_mode(regs))
>>  		_exception(sig, regs, code, regs->dar);
>>  	else
>> -		bad_page_fault(regs, regs->dar, sig);
>> +		bad_page_fault(regs, sig);
>>
>>  bail:
>>  	exception_exit(prev_state);
>> diff --git a/arch/powerpc/mm/book3s64/hash_utils.c  
>> b/arch/powerpc/mm/book3s64/hash_utils.c
>> index 0f0bd4af4b2d..731518e7d56f 100644
>> --- a/arch/powerpc/mm/book3s64/hash_utils.c
>> +++ b/arch/powerpc/mm/book3s64/hash_utils.c
>> @@ -1537,7 +1537,7 @@ long do_hash_fault(struct pt_regs *regs)
>>  	 * the access, or panic if there isn't a handler.
>>  	 */
>>  	if (unlikely(in_nmi())) {
>> -		bad_page_fault(regs, ea, SIGSEGV);
>> +		bad_page_fault(regs, SIGSEGV);
>>  		return 0;
>>  	}
>>
>> @@ -1576,7 +1576,7 @@ long do_hash_fault(struct pt_regs *regs)
>>  			else
>>  				_exception(SIGBUS, regs, BUS_ADRERR, ea);
>>  		} else {
>> -			bad_page_fault(regs, ea, SIGBUS);
>> +			bad_page_fault(regs, SIGBUS);
>>  		}
>>  		err = 0;
>>
>> diff --git a/arch/powerpc/mm/book3s64/slb.c b/arch/powerpc/mm/book3s64/slb.c
>> index cc34d50874c1..ae89ad516247 100644
>> --- a/arch/powerpc/mm/book3s64/slb.c
>> +++ b/arch/powerpc/mm/book3s64/slb.c
>> @@ -898,7 +898,7 @@ void do_bad_slb_fault(struct pt_regs *regs)
>>  		if (user_mode(regs))
>>  			_exception(SIGSEGV, regs, SEGV_BNDERR, regs->dar);
>>  		else
>> -			bad_page_fault(regs, regs->dar, SIGSEGV);
>> +			bad_page_fault(regs, SIGSEGV);
>>  	} else if (err == -EINVAL) {
>>  		unrecoverable_exception(regs);
>>  	} else {
>> diff --git a/arch/powerpc/mm/fault.c b/arch/powerpc/mm/fault.c
>> index 390a296b16a3..db73b373e76c 100644
>> --- a/arch/powerpc/mm/fault.c
>> +++ b/arch/powerpc/mm/fault.c
>> @@ -375,7 +375,7 @@ static void sanity_check_fault(bool is_write,  
>> bool is_user,
>>  #elif defined(CONFIG_PPC_BOOK3E_64)
>>  #define page_fault_is_bad(__err)	((__err) & DSISR_BAD_FAULT_64S)
>>  #else
>> -#define page_fault_is_bad(__err)	((__err) & DSISR_BAD_FAULT_32S)
>> +#define page_fault_is_bad(__err)	((__err) & (DSISR_BAD_FAULT_32S |  
>> DSISR_DABRMATCH))
>>  #endif
>>  #endif
>>
>> @@ -408,7 +408,7 @@ static int __do_page_fault(struct pt_regs *regs,  
>> unsigned long address,
>>  		return 0;
>>
>>  	if (unlikely(page_fault_is_bad(error_code))) {
>> -		if (IS_ENABLED(CONFIG_PPC_BOOK3S_64) && (error_code & DSISR_DABRMATCH))
>> +		if (IS_ENABLED(CONFIG_PPC_BOOK3S) && (error_code & DSISR_DABRMATCH))
>>  			return -1;
>>
>>  		if (is_user) {
>> @@ -562,14 +562,14 @@ long do_page_fault(struct pt_regs *regs)
>>  	/* 32 and 64e handle errors in their asm code */
>>  	if (unlikely(err)) {
>>  		if (err > 0) {
>> -			bad_page_fault(regs, address, err);
>> +			bad_page_fault(regs, err);
>>  			err = 0;
>>  		} else {
>>  			/*
>>  			 * do_break() may change NV GPRS while handling the
>>  			 * breakpoint. Return -ve to caller to do that.
>>  			 */
>> -			do_break(regs, address, error_code);
>> +			do_break(regs);
>>  		}
>>  	}
>>  #endif
>> @@ -591,14 +591,14 @@ long hash__do_page_fault(struct pt_regs *regs)
>>  	err = __do_page_fault(regs, address, error_code);
>>  	if (unlikely(err)) {
>>  		if (err > 0) {
>> -			bad_page_fault(regs, address, err);
>> +			bad_page_fault(regs, err);
>>  			err = 0;
>>  		} else {
>>  			/*
>>  			 * do_break() may change NV GPRS while handling the
>>  			 * breakpoint. Return -ve to caller to do that.
>>  			 */
>> -			do_break(regs, address, error_code);
>> +			do_break(regs);
>>  		}
>>  	}
>>
>> @@ -612,7 +612,7 @@ NOKPROBE_SYMBOL(hash__do_page_fault);
>>   * It is called from the DSI and ISI handlers in head.S and from some
>>   * of the procedures in traps.c.
>>   */
>> -void bad_page_fault(struct pt_regs *regs, unsigned long address, int sig)
>> +void bad_page_fault(struct pt_regs *regs, int sig)
>>  {
>>  	const struct exception_table_entry *entry;
>>  	int is_write = page_fault_is_write(regs->dsisr);
>> diff --git a/arch/powerpc/platforms/8xx/machine_check.c  
>> b/arch/powerpc/platforms/8xx/machine_check.c
>> index 88dedf38eccd..656365975895 100644
>> --- a/arch/powerpc/platforms/8xx/machine_check.c
>> +++ b/arch/powerpc/platforms/8xx/machine_check.c
>> @@ -26,7 +26,7 @@ int machine_check_8xx(struct pt_regs *regs)
>>  	 * to deal with that than having a wart in the mcheck handler.
>>  	 * -- BenH
>>  	 */
>> -	bad_page_fault(regs, regs->dar, SIGBUS);
>> +	bad_page_fault(regs, SIGBUS);
>>  	return 1;
>>  #else
>>  	return 0;
>> --
>> 2.23.0
> 
> 
> 


More information about the Linuxppc-dev mailing list