From: Will Deacon <will@kernel.org>
To: Mark Brown <broonie@kernel.org>
Cc: Catalin Marinas <catalin.marinas@arm.com>,
linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH 1/2] arm64: kernel: Convert to modern annotations for assembly functions
Date: Thu, 30 Apr 2020 18:18:25 +0100 [thread overview]
Message-ID: <20200430171825.GI25258@willie-the-truck> (raw)
In-Reply-To: <20200428164331.27289-2-broonie@kernel.org>
On Tue, Apr 28, 2020 at 05:43:30PM +0100, Mark Brown wrote:
> In an effort to clarify and simplify the annotation of assembly functions
> in the kernel new macros have been introduced. These replace ENTRY and
> ENDPROC and also add a new annotation for static functions which previously
> had no ENTRY equivalent. Update the annotations in the core kernel code to
> the new macros.
>
> Signed-off-by: Mark Brown <broonie@kernel.org>
> ---
> arch/arm64/kernel/cpu-reset.S | 4 +-
> arch/arm64/kernel/efi-rt-wrapper.S | 4 +-
> arch/arm64/kernel/entry-fpsimd.S | 20 ++++-----
> arch/arm64/kernel/entry.S | 7 +--
> arch/arm64/kernel/hibernate-asm.S | 16 +++----
> arch/arm64/kernel/hyp-stub.S | 20 ++++-----
> arch/arm64/kernel/probes/kprobes_trampoline.S | 4 +-
> arch/arm64/kernel/reloc_test_syms.S | 44 +++++++++----------
> arch/arm64/kernel/relocate_kernel.S | 4 +-
> arch/arm64/kernel/sleep.S | 12 ++---
> arch/arm64/kernel/smccc-call.S | 8 ++--
> 11 files changed, 72 insertions(+), 71 deletions(-)
>
> diff --git a/arch/arm64/kernel/cpu-reset.S b/arch/arm64/kernel/cpu-reset.S
> index 38087b4c0432..646103743a50 100644
> --- a/arch/arm64/kernel/cpu-reset.S
> +++ b/arch/arm64/kernel/cpu-reset.S
> @@ -29,7 +29,7 @@
> * branch to what would be the reset vector. It must be executed with the
> * flat identity mapping.
> */
> -ENTRY(__cpu_soft_restart)
> +SYM_FUNC_START(__cpu_soft_restart)
> /* Clear sctlr_el1 flags. */
> mrs x12, sctlr_el1
> mov_q x13, SCTLR_ELx_FLAGS
> @@ -47,6 +47,6 @@ ENTRY(__cpu_soft_restart)
> mov x1, x3 // arg1
> mov x2, x4 // arg2
> br x8
> -ENDPROC(__cpu_soft_restart)
> +SYM_FUNC_END(__cpu_soft_restart)
Hmm, this function is probably weird enough to justify SYM_CODE_* since it
never returns and doesn't have a stack.
> diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
> index ddcde093c433..664a833aa619 100644
> --- a/arch/arm64/kernel/entry.S
> +++ b/arch/arm64/kernel/entry.S
> @@ -730,7 +730,7 @@ SYM_CODE_END(el0_error)
> /*
> * Ok, we need to do extra processing, enter the slow path.
> */
> -work_pending:
> +SYM_CODE_START_LOCAL(work_pending)
> mov x0, sp // 'regs'
> bl do_notify_resume
> #ifdef CONFIG_TRACE_IRQFLAGS
> @@ -738,10 +738,11 @@ work_pending:
> #endif
> ldr x1, [tsk, #TSK_TI_FLAGS] // re-check for single-step
> b finish_ret_to_user
> +SYM_CODE_END(work_pending)
> /*
> * "slow" syscall return path.
> */
> -ret_to_user:
> +SYM_CODE_START_LOCAL(ret_to_user)
Would this be better off as a SYM_INNER_LABEL inside work_pending? Given
that ret_to_user and work_pending both branch into each other, separating
them doesn't feel quite right.
> diff --git a/arch/arm64/kernel/hibernate-asm.S b/arch/arm64/kernel/hibernate-asm.S
> index 6532105b3e32..8ccca660034e 100644
> --- a/arch/arm64/kernel/hibernate-asm.S
> +++ b/arch/arm64/kernel/hibernate-asm.S
[...]
> .macro invalid_vector label
> -\label:
> +SYM_CODE_START_LOCAL(\label)
> b \label
> -ENDPROC(\label)
> +SYM_CODE_END(\label)
> .endm
[...]
> diff --git a/arch/arm64/kernel/hyp-stub.S b/arch/arm64/kernel/hyp-stub.S
> index e473ead806ed..160f5881a0b7 100644
> --- a/arch/arm64/kernel/hyp-stub.S
> +++ b/arch/arm64/kernel/hyp-stub.S
[...]
> .macro invalid_vector label
> -\label:
> +SYM_CODE_START_LOCAL(\label)
> b \label
> -ENDPROC(\label)
> +SYM_CODE_END(\label)
> .endm
Huh, this is the exact same macro as the one from the hibernate code. Maybe
we should stick it in asm/asembler.h alongside ventry? Obviously a separate
patch, though.
> diff --git a/arch/arm64/kernel/probes/kprobes_trampoline.S b/arch/arm64/kernel/probes/kprobes_trampoline.S
> index 45dce03aaeaf..890ca72c5a51 100644
> --- a/arch/arm64/kernel/probes/kprobes_trampoline.S
> +++ b/arch/arm64/kernel/probes/kprobes_trampoline.S
> @@ -61,7 +61,7 @@
> ldp x28, x29, [sp, #S_X28]
> .endm
>
> -ENTRY(kretprobe_trampoline)
> +SYM_CODE_START(kretprobe_trampoline)
> sub sp, sp, #S_FRAME_SIZE
>
> save_all_base_regs
> @@ -79,4 +79,4 @@ ENTRY(kretprobe_trampoline)
> add sp, sp, #S_FRAME_SIZE
> ret
>
> -ENDPROC(kretprobe_trampoline)
> +SYM_CODE_END(kretprobe_trampoline)
> diff --git a/arch/arm64/kernel/reloc_test_syms.S b/arch/arm64/kernel/reloc_test_syms.S
> index 16a34f188f26..53e8cdfe80e1 100644
> --- a/arch/arm64/kernel/reloc_test_syms.S
> +++ b/arch/arm64/kernel/reloc_test_syms.S
> @@ -5,81 +5,81 @@
>
> #include <linux/linkage.h>
>
> -ENTRY(absolute_data64)
> +SYM_CODE_START(absolute_data64)
> ldr x0, 0f
> ret
> 0: .quad sym64_abs
> -ENDPROC(absolute_data64)
> +SYM_CODE_END(absolute_data64)
Hmm, but all the functions in here *are* actually just called from the C
code in reloc_test_core.c afaict, so they should probably be using
SYM_FUNC_*.
> diff --git a/arch/arm64/kernel/relocate_kernel.S b/arch/arm64/kernel/relocate_kernel.S
> index c40ce496c78b..542d6edc6806 100644
> --- a/arch/arm64/kernel/relocate_kernel.S
> +++ b/arch/arm64/kernel/relocate_kernel.S
> @@ -26,7 +26,7 @@
> * control_code_page, a special page which has been set up to be preserved
> * during the copy operation.
> */
> -ENTRY(arm64_relocate_new_kernel)
> +SYM_CODE_START(arm64_relocate_new_kernel)
>
> /* Setup the list loop variables. */
> mov x18, x2 /* x18 = dtb address */
> @@ -111,7 +111,7 @@ ENTRY(arm64_relocate_new_kernel)
> mov x3, xzr
> br x17
>
> -ENDPROC(arm64_relocate_new_kernel)
> +SYM_CODE_END(arm64_relocate_new_kernel)
>
> .align 3 /* To keep the 64-bit values below naturally aligned. */
>
> diff --git a/arch/arm64/kernel/sleep.S b/arch/arm64/kernel/sleep.S
> index 7b2f2e650c44..70e5e697ebc7 100644
> --- a/arch/arm64/kernel/sleep.S
> +++ b/arch/arm64/kernel/sleep.S
> @@ -62,7 +62,7 @@
> *
> * x0 = struct sleep_stack_data area
> */
> -ENTRY(__cpu_suspend_enter)
> +SYM_FUNC_START(__cpu_suspend_enter)
> stp x29, lr, [x0, #SLEEP_STACK_DATA_CALLEE_REGS]
> stp x19, x20, [x0,#SLEEP_STACK_DATA_CALLEE_REGS+16]
> stp x21, x22, [x0,#SLEEP_STACK_DATA_CALLEE_REGS+32]
> @@ -95,10 +95,10 @@ ENTRY(__cpu_suspend_enter)
> ldp x29, lr, [sp], #16
> mov x0, #1
> ret
> -ENDPROC(__cpu_suspend_enter)
> +SYM_FUNC_END(__cpu_suspend_enter)
>
> .pushsection ".idmap.text", "awx"
> -ENTRY(cpu_resume)
> +SYM_FUNC_START(cpu_resume)
> bl el2_setup // if in EL2 drop to EL1 cleanly
> mov x0, #ARM64_CPU_RUNTIME
> bl __cpu_setup
> @@ -107,11 +107,11 @@ ENTRY(cpu_resume)
> bl __enable_mmu
> ldr x8, =_cpu_resume
> br x8
> -ENDPROC(cpu_resume)
> +SYM_FUNC_END(cpu_resume)
SYM_CODE_* here, as this is I think this is the entry point from the resume
path?
Will
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2020-04-30 17:18 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-04-28 16:43 [PATCH 0/2] arm64: Finish up assembler annotation modernisation Mark Brown
2020-04-28 16:43 ` [PATCH 1/2] arm64: kernel: Convert to modern annotations for assembly functions Mark Brown
2020-04-30 17:18 ` Will Deacon [this message]
2020-04-30 18:12 ` Mark Brown
2020-04-28 16:43 ` [PATCH 2/2] arm64: Disable old style assembly annotations Mark Brown
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200430171825.GI25258@willie-the-truck \
--to=will@kernel.org \
--cc=broonie@kernel.org \
--cc=catalin.marinas@arm.com \
--cc=linux-arm-kernel@lists.infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).