Re: [PATCH 07/17] arm64: entry: convert error entry to C

From: Anshuman Khandual <anshuman.khandual@arm.com>
To: Mark Rutland <mark.rutland@arm.com>,
	linux-arm-kernel@lists.infradead.org, catalin.marinas@arm.com,
	will@kernel.org, james.morse@arm.com
Cc: keescook@chromium.org, maz@kernel.org, broonie@kernel.org,
	labbott@redhat.com, robin.murphy@arm.com,
	julien.thierry.kdev@gmail.com, alex.popov@linux.com
Subject: Re: [PATCH 07/17] arm64: entry: convert error entry to C
Date: Thu, 9 Jan 2020 14:42:23 +0530	[thread overview]
Message-ID: <71785264-d196-b84c-d527-1a49f8aa7dda@arm.com> (raw)
In-Reply-To: <20200108185634.1163-8-mark.rutland@arm.com>

On 01/09/2020 12:26 AM, Mark Rutland wrote:
> Portions of our error entry logic are handled in C while other parts are
> handled in assembly. Let's migrate the bulk of it to C so that it's
> easier to follow and maintain.
> 
> Signed-off-by: Mark Rutland <mark.rutland@arm.com>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Cc: James Morse <james.morse@arm.com>
> Cc: Will Deacon <will@kernel.org>
> ---
>  arch/arm64/include/asm/exception.h |  1 +
>  arch/arm64/kernel/entry-common.c   | 26 ++++++++++++++++++++++++++
>  arch/arm64/kernel/entry.S          | 19 ++++++-------------
>  arch/arm64/kernel/traps.c          |  2 +-
>  4 files changed, 34 insertions(+), 14 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/exception.h b/arch/arm64/include/asm/exception.h
> index a49038fa4faf..220a7c3971d8 100644
> --- a/arch/arm64/include/asm/exception.h
> +++ b/arch/arm64/include/asm/exception.h
> @@ -51,5 +51,6 @@ void do_el0_svc(struct pt_regs *regs);
>  void do_el0_svc_compat(struct pt_regs *regs);
>  void do_el0_ia_bp_hardening(unsigned long addr,  unsigned int esr,
>  			    struct pt_regs *regs);
> +void do_serror(struct pt_regs *regs, unsigned int esr);
>  
>  #endif	/* __ASM_EXCEPTION_H */
> diff --git a/arch/arm64/kernel/entry-common.c b/arch/arm64/kernel/entry-common.c
> index 45155d9f72da..bf9d497e620c 100644
> --- a/arch/arm64/kernel/entry-common.c
> +++ b/arch/arm64/kernel/entry-common.c
> @@ -442,3 +442,29 @@ asmlinkage void notrace el0_irq_handler(struct pt_regs *regs)
>  	trace_hardirqs_on();
>  }
>  NOKPROBE_SYMBOL(el0_irq_handler);
> +
> +asmlinkage void el1_error_handler(struct pt_regs *regs)
> +{
> +	unsigned long esr = read_sysreg(esr_el1);
> +
> +	if (system_uses_irq_prio_masking())
> +		gic_write_pmr(GIC_PRIO_IRQON | GIC_PRIO_PSR_I_SET);
> +
> +	local_daif_restore(DAIF_ERRCTX);
> +	do_serror(regs, esr);
> +}
> +NOKPROBE_SYMBOL(el1_error_handler);
> +
> +asmlinkage void el0_error_handler(struct pt_regs *regs)
> +{
> +	unsigned long esr = read_sysreg(esr_el1);
> +
> +	if (system_uses_irq_prio_masking())
> +		gic_write_pmr(GIC_PRIO_IRQON | GIC_PRIO_PSR_I_SET);
> +
> +	user_exit_irqoff();
> +	local_daif_restore(DAIF_ERRCTX);

Just being curious. local_daif_restore(DAIF_ERRCTX) has the same
effect as enable_dbg asm macro previously ?

> +	do_serror(regs, esr);
> +	local_daif_restore(DAIF_PROCCTX_NOIRQ);
> +}
> +NOKPROBE_SYMBOL(el0_error_handler);
> diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
> index 55c4be1e996a..0c5117ef7c3c 100644
> --- a/arch/arm64/kernel/entry.S
> +++ b/arch/arm64/kernel/entry.S
> @@ -559,7 +559,10 @@ ENDPROC(el0_irq_compat)
>  
>  el0_error_compat:
>  	kernel_entry 0, 32
> -	b	el0_error_naked
> +	mov	x0, sp
> +	bl	el0_error_handler
> +	b	ret_to_user
> +ENDPROC(el0_error_compat)
>  #endif
>  
>  	.align	6
> @@ -572,25 +575,15 @@ ENDPROC(el0_irq)
>  
>  el1_error:
>  	kernel_entry 1
> -	mrs	x1, esr_el1
> -	gic_prio_kentry_setup tmp=x2
> -	enable_dbg
>  	mov	x0, sp
> -	bl	do_serror
> +	bl	el1_error_handler
>  	kernel_exit 1
>  ENDPROC(el1_error)
>  
>  el0_error:
>  	kernel_entry 0
> -el0_error_naked:
> -	mrs	x25, esr_el1
> -	gic_prio_kentry_setup tmp=x2
> -	ct_user_exit_irqoff
> -	enable_dbg
>  	mov	x0, sp
> -	mov	x1, x25
> -	bl	do_serror
> -	enable_da_f
> +	bl	el0_error_handler
>  	b	ret_to_user
>  ENDPROC(el0_error)

Macros enable_da_f and ct_user_exit_irqoff can be removed here itself
although it is eventually getting dropped off in a later patch.

>  
> diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
> index 73caf35c2262..170e637bb87b 100644
> --- a/arch/arm64/kernel/traps.c
> +++ b/arch/arm64/kernel/traps.c
> @@ -901,7 +901,7 @@ bool arm64_is_fatal_ras_serror(struct pt_regs *regs, unsigned int esr)
>  	}
>  }
>  
> -asmlinkage void do_serror(struct pt_regs *regs, unsigned int esr)
> +void do_serror(struct pt_regs *regs, unsigned int esr)
>  {
>  	const bool was_in_nmi = in_nmi();
>  
> 

Should not we add NOKPROBE_SYMBOL() for the symbol.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel