| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <aJsfB3DJCduz6lzx@J2N7QTR9R3>
Date: Tue, 12 Aug 2025 12:01:27 +0100
From: Mark Rutland <mark.rutland@....com>
To: Jinjie Ruan <ruanjinjie@...wei.com>
Cc: catalin.marinas@....com, will@...nel.org, oleg@...hat.com,
sstabellini@...nel.org, puranjay@...nel.org, broonie@...nel.org,
mbenes@...e.cz, ryan.roberts@....com, akpm@...ux-foundation.org,
chenl311@...natelecom.cn, ada.coupriediaz@....com,
anshuman.khandual@....com, kristina.martsenko@....com,
liaochang1@...wei.com, ardb@...nel.org, leitao@...ian.org,
linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
xen-devel@...ts.xenproject.org
Subject: Re: [PATCH -next v7 2/7] arm64: entry: Refactor the entry and exit
for exceptions from EL1
Hi Jinjie,
On Tue, Jul 29, 2025 at 09:54:51AM +0800, Jinjie Ruan wrote:
> The generic entry code uses irqentry_state_t to track lockdep and RCU
> state across exception entry and return. For historical reasons, arm64
> embeds similar fields within its pt_regs structure.
>
> In preparation for moving arm64 over to the generic entry code, pull
> these fields out of arm64's pt_regs, and use a separate structure,
> matching the style of the generic entry code.
>
> No functional changes.
>
> Suggested-by: Mark Rutland <mark.rutland@....com>
> Signed-off-by: Jinjie Ruan <ruanjinjie@...wei.com>
One minor formatting nit below, but with aside from that this looks
great, and with that fixed up:
Acked-by: Mark Rutland <mark.rutland@....com>
[...]
> @@ -475,73 +497,81 @@ UNHANDLED(el1t, 64, error)
> static void noinstr el1_abort(struct pt_regs *regs, unsigned long esr)
> {
> unsigned long far = read_sysreg(far_el1);
> + arm64_irqentry_state_t state;
>
> - enter_from_kernel_mode(regs);
> + state = enter_from_kernel_mode(regs);
> local_daif_inherit(regs);
> do_mem_abort(far, esr, regs);
> local_daif_mask();
> - exit_to_kernel_mode(regs);
> + exit_to_kernel_mode(regs, state);
> }
>
> static void noinstr el1_pc(struct pt_regs *regs, unsigned long esr)
> {
> unsigned long far = read_sysreg(far_el1);
> + arm64_irqentry_state_t state;
>
> - enter_from_kernel_mode(regs);
> + state = enter_from_kernel_mode(regs);
> local_daif_inherit(regs);
> do_sp_pc_abort(far, esr, regs);
> local_daif_mask();
> - exit_to_kernel_mode(regs);
> + exit_to_kernel_mode(regs, state);
> }
>
> static void noinstr el1_undef(struct pt_regs *regs, unsigned long esr)
> {
> - enter_from_kernel_mode(regs);
> + arm64_irqentry_state_t state = enter_from_kernel_mode(regs);
> +
> local_daif_inherit(regs);
> do_el1_undef(regs, esr);
> local_daif_mask();
> - exit_to_kernel_mode(regs);
> + exit_to_kernel_mode(regs, state);
> }
I'd prefer if we consistently defined 'state' on a separate line, before
the main block consisting of:
state = enter_from_kernel_mode(regs);
local_daif_inherit(regs);
do_el1_undef(regs, esr);
local_daif_mask();
exit_to_kernel_mode(regs, state);
... since that way the enter/exit functions clearly enclose the whole
block, which isn't as clear when there's a line gap between
enter_from_kernel_mode() and the rest of the block.
That would also be more consistent with what we do for functions that
need to read other registers (e.g. el1_abort() and el1_pc() above).
If that could be applied consistently here and below, that'd be great.
Mark.
> static void noinstr el1_bti(struct pt_regs *regs, unsigned long esr)
> {
> - enter_from_kernel_mode(regs);
> + arm64_irqentry_state_t state = enter_from_kernel_mode(regs);
> +
> local_daif_inherit(regs);
> do_el1_bti(regs, esr);
> local_daif_mask();
> - exit_to_kernel_mode(regs);
> + exit_to_kernel_mode(regs, state);
> }
>
> static void noinstr el1_gcs(struct pt_regs *regs, unsigned long esr)
> {
> - enter_from_kernel_mode(regs);
> + arm64_irqentry_state_t state = enter_from_kernel_mode(regs);
> +
> local_daif_inherit(regs);
> do_el1_gcs(regs, esr);
> local_daif_mask();
> - exit_to_kernel_mode(regs);
> + exit_to_kernel_mode(regs, state);
> }
>
> static void noinstr el1_mops(struct pt_regs *regs, unsigned long esr)
> {
> - enter_from_kernel_mode(regs);
> + arm64_irqentry_state_t state = enter_from_kernel_mode(regs);
> +
> local_daif_inherit(regs);
> do_el1_mops(regs, esr);
> local_daif_mask();
> - exit_to_kernel_mode(regs);
> + exit_to_kernel_mode(regs, state);
> }
>
> static void noinstr el1_breakpt(struct pt_regs *regs, unsigned long esr)
> {
> - arm64_enter_el1_dbg(regs);
> + arm64_irqentry_state_t state = arm64_enter_el1_dbg(regs);
> +
> debug_exception_enter(regs);
> do_breakpoint(esr, regs);
> debug_exception_exit(regs);
> - arm64_exit_el1_dbg(regs);
> + arm64_exit_el1_dbg(regs, state);
> }
>
> static void noinstr el1_softstp(struct pt_regs *regs, unsigned long esr)
> {
> - arm64_enter_el1_dbg(regs);
> + arm64_irqentry_state_t state = arm64_enter_el1_dbg(regs);
> +
> if (!cortex_a76_erratum_1463225_debug_handler(regs)) {
> debug_exception_enter(regs);
> /*
> @@ -554,37 +584,40 @@ static void noinstr el1_softstp(struct pt_regs *regs, unsigned long esr)
> do_el1_softstep(esr, regs);
> debug_exception_exit(regs);
> }
> - arm64_exit_el1_dbg(regs);
> + arm64_exit_el1_dbg(regs, state);
> }
>
> static void noinstr el1_watchpt(struct pt_regs *regs, unsigned long esr)
> {
> /* Watchpoints are the only debug exception to write FAR_EL1 */
> unsigned long far = read_sysreg(far_el1);
> + arm64_irqentry_state_t state;
>
> - arm64_enter_el1_dbg(regs);
> + state = arm64_enter_el1_dbg(regs);
> debug_exception_enter(regs);
> do_watchpoint(far, esr, regs);
> debug_exception_exit(regs);
> - arm64_exit_el1_dbg(regs);
> + arm64_exit_el1_dbg(regs, state);
> }
>
> static void noinstr el1_brk64(struct pt_regs *regs, unsigned long esr)
> {
> - arm64_enter_el1_dbg(regs);
> + arm64_irqentry_state_t state = arm64_enter_el1_dbg(regs);
> +
> debug_exception_enter(regs);
> do_el1_brk64(esr, regs);
> debug_exception_exit(regs);
> - arm64_exit_el1_dbg(regs);
> + arm64_exit_el1_dbg(regs, state);
> }
>
> static void noinstr el1_fpac(struct pt_regs *regs, unsigned long esr)
> {
> - enter_from_kernel_mode(regs);
> + arm64_irqentry_state_t state = enter_from_kernel_mode(regs);
> +
> local_daif_inherit(regs);
> do_el1_fpac(regs, esr);
> local_daif_mask();
> - exit_to_kernel_mode(regs);
> + exit_to_kernel_mode(regs, state);
> }
>
> asmlinkage void noinstr el1h_64_sync_handler(struct pt_regs *regs)
> @@ -639,15 +672,16 @@ asmlinkage void noinstr el1h_64_sync_handler(struct pt_regs *regs)
> static __always_inline void __el1_pnmi(struct pt_regs *regs,
> void (*handler)(struct pt_regs *))
> {
> - arm64_enter_nmi(regs);
> + arm64_irqentry_state_t state = arm64_enter_nmi(regs);
> +
> do_interrupt_handler(regs, handler);
> - arm64_exit_nmi(regs);
> + arm64_exit_nmi(regs, state);
> }
>
> static __always_inline void __el1_irq(struct pt_regs *regs,
> void (*handler)(struct pt_regs *))
> {
> - enter_from_kernel_mode(regs);
> + arm64_irqentry_state_t state = enter_from_kernel_mode(regs);
>
> irq_enter_rcu();
> do_interrupt_handler(regs, handler);
> @@ -655,7 +689,7 @@ static __always_inline void __el1_irq(struct pt_regs *regs,
>
> arm64_preempt_schedule_irq();
>
> - exit_to_kernel_mode(regs);
> + exit_to_kernel_mode(regs, state);
> }
> static void noinstr el1_interrupt(struct pt_regs *regs,
> void (*handler)(struct pt_regs *))
> @@ -681,11 +715,12 @@ asmlinkage void noinstr el1h_64_fiq_handler(struct pt_regs *regs)
> asmlinkage void noinstr el1h_64_error_handler(struct pt_regs *regs)
> {
> unsigned long esr = read_sysreg(esr_el1);
> + arm64_irqentry_state_t state;
>
> local_daif_restore(DAIF_ERRCTX);
> - arm64_enter_nmi(regs);
> + state = arm64_enter_nmi(regs);
> do_serror(regs, esr);
> - arm64_exit_nmi(regs);
> + arm64_exit_nmi(regs, state);
> }
>
> static void noinstr el0_da(struct pt_regs *regs, unsigned long esr)
> @@ -997,12 +1032,13 @@ asmlinkage void noinstr el0t_64_fiq_handler(struct pt_regs *regs)
> static void noinstr __el0_error_handler_common(struct pt_regs *regs)
> {
> unsigned long esr = read_sysreg(esr_el1);
> + arm64_irqentry_state_t state;
>
> enter_from_user_mode(regs);
> local_daif_restore(DAIF_ERRCTX);
> - arm64_enter_nmi(regs);
> + state = arm64_enter_nmi(regs);
> do_serror(regs, esr);
> - arm64_exit_nmi(regs);
> + arm64_exit_nmi(regs, state);
> local_daif_restore(DAIF_PROCCTX);
> exit_to_user_mode(regs);
> }
> @@ -1122,6 +1158,7 @@ asmlinkage void noinstr __noreturn handle_bad_stack(struct pt_regs *regs)
> asmlinkage noinstr unsigned long
> __sdei_handler(struct pt_regs *regs, struct sdei_registered_event *arg)
> {
> + arm64_irqentry_state_t state;
> unsigned long ret;
>
> /*
> @@ -1146,9 +1183,9 @@ __sdei_handler(struct pt_regs *regs, struct sdei_registered_event *arg)
> else if (cpu_has_pan())
> set_pstate_pan(0);
>
> - arm64_enter_nmi(regs);
> + state = arm64_enter_nmi(regs);
> ret = do_sdei_event(regs, arg);
> - arm64_exit_nmi(regs);
> + arm64_exit_nmi(regs, state);
>
> return ret;
> }
> --
> 2.34.1
>
Powered by blists - more mailing lists