| Message ID | 20200108185634.1163-8-mark.rutland@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | arm64: entry deasmification and cleanup | expand |
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.
On Thu, Jan 09, 2020 at 02:42:23PM +0530, Anshuman Khandual wrote: > > > 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 ? Not quite, DAIF_ERRCTX unmasks debug and FIQ, but I beleive that was an oversight when we tightened up the DAIF nesting rules bask in commit: 65be7a1b799f11ff ("arm64: introduce an order for exceptions") ... where the commit message says: | FIQ is never expected, but we mask it when we mask debug exceptions, | and unmask it at all other times. I'll call that out explicitly in the commit message. > > + 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. True. I'll fold those removals here. I'll prepend a patch to remove inherit_daif, too. > > 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. At this point we've already sampled the exception registers and balanced the IRQ flags, so we don't need to do so for the sames reasons as for functions in entry-common.c. If that needs to change, I think that should be a separate patch, as nothing has changed from the PoV of do_serror() other than asmlinkage. Thanks, Mark.
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); + 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) 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();
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(-)