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, jslaby@suse.com
Subject: Re: [PATCH 2/3] arm64: lib: Use modern annotations for assembly functions
Date: Tue, 7 Jan 2020 14:44:46 +0000 [thread overview]
Message-ID: <20200107144445.GC29001@willie-the-truck> (raw)
In-Reply-To: <20200106195818.56351-3-broonie@kernel.org>
Hi Mark,
[+Jiri -- couple of questions below]
On Mon, Jan 06, 2020 at 07:58:17PM +0000, 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 library code to the
> new macros.
>
> Signed-off-by: Mark Brown <broonie@kernel.org>
> ---
> arch/arm64/lib/clear_page.S | 4 ++--
> arch/arm64/lib/clear_user.S | 4 ++--
> arch/arm64/lib/copy_from_user.S | 4 ++--
> arch/arm64/lib/copy_in_user.S | 4 ++--
> arch/arm64/lib/copy_page.S | 4 ++--
> arch/arm64/lib/copy_to_user.S | 4 ++--
> arch/arm64/lib/crc32.S | 8 ++++----
> arch/arm64/lib/memchr.S | 4 ++--
> arch/arm64/lib/memcmp.S | 4 ++--
> arch/arm64/lib/memcpy.S | 8 ++++----
> arch/arm64/lib/memmove.S | 8 ++++----
> arch/arm64/lib/memset.S | 8 ++++----
> arch/arm64/lib/strchr.S | 4 ++--
> arch/arm64/lib/strcmp.S | 4 ++--
> arch/arm64/lib/strlen.S | 4 ++--
> arch/arm64/lib/strncmp.S | 4 ++--
> arch/arm64/lib/strnlen.S | 4 ++--
> arch/arm64/lib/strrchr.S | 4 ++--
> arch/arm64/lib/tishift.S | 12 ++++++------
> 19 files changed, 50 insertions(+), 50 deletions(-)
>
> diff --git a/arch/arm64/lib/clear_page.S b/arch/arm64/lib/clear_page.S
> index 78a9ef66288a..073acbf02a7c 100644
> --- a/arch/arm64/lib/clear_page.S
> +++ b/arch/arm64/lib/clear_page.S
> @@ -14,7 +14,7 @@
> * Parameters:
> * x0 - dest
> */
> -ENTRY(clear_page)
> +SYM_FUNC_START(clear_page)
> mrs x1, dczid_el0
> and w1, w1, #0xf
> mov x2, #4
Since this doesn't change behaviour, I think the patch is fine, however
on reading Documentation/asm-annotations.rst it's not completely clear to
me when SYM_FUNC_START() should be used. In this case, for example, we are
*not* pushing a stackframe and that would appear to be at odds with the
documentation.
Jiri -- is it ok to omit the stack frame for leaf functions annotated with
SYM_FUNC_START? I'm guessing it should be, since the link register isn't
going to be clobbered. Could we update the documentation to reflect that?
> diff --git a/arch/arm64/lib/crc32.S b/arch/arm64/lib/crc32.S
> index e6135f16649b..243e107e9896 100644
> --- a/arch/arm64/lib/crc32.S
> +++ b/arch/arm64/lib/crc32.S
> @@ -85,17 +85,17 @@ CPU_BE( rev16 w3, w3 )
> .endm
>
> .align 5
> -ENTRY(crc32_le)
> +SYM_FUNC_START(crc32_le)
> alternative_if_not ARM64_HAS_CRC32
> b crc32_le_base
Similar thing here -- I'm assuming we are ok to tail-call crc32_le_base()
from a function marked with SYM_FUNC_START like this?
> alternative_else_nop_endif
> __crc32
> -ENDPROC(crc32_le)
> +SYM_FUNC_END(crc32_le)
>
> .align 5
> -ENTRY(__crc32c_le)
> +SYM_FUNC_START(__crc32c_le)
> alternative_if_not ARM64_HAS_CRC32
> b __crc32c_le_base
> alternative_else_nop_endif
> __crc32 c
> -ENDPROC(__crc32c_le)
> +SYM_FUNC_END(__crc32c_le)
> diff --git a/arch/arm64/lib/memchr.S b/arch/arm64/lib/memchr.S
> index 48a3ab636e4f..99910c5d5db7 100644
> --- a/arch/arm64/lib/memchr.S
> +++ b/arch/arm64/lib/memchr.S
> @@ -19,7 +19,7 @@
> * Returns:
> * x0 - address of first occurrence of 'c' or 0
> */
> -WEAK(memchr)
> +SYM_FUNC_START_PI_WEAK(memchr)
> and w1, w1, #0xff
> 1: subs x2, x2, #1
> b.mi 2f
> @@ -30,5 +30,5 @@ WEAK(memchr)
> ret
> 2: mov x0, #0
> ret
> -ENDPIPROC(memchr)
> +SYM_FUNC_END_PI(memchr)
> EXPORT_SYMBOL_NOKASAN(memchr)
> diff --git a/arch/arm64/lib/memcmp.S b/arch/arm64/lib/memcmp.S
> index b297bdaaf549..b889f312bdb3 100644
> --- a/arch/arm64/lib/memcmp.S
> +++ b/arch/arm64/lib/memcmp.S
> @@ -46,7 +46,7 @@ pos .req x11
> limit_wd .req x12
> mask .req x13
>
> -WEAK(memcmp)
> +SYM_FUNC_START_PI_WEAK(memcmp)
> cbz limit, .Lret0
> eor tmp1, src1, src2
> tst tmp1, #7
> @@ -243,5 +243,5 @@ CPU_LE( rev data2, data2 )
> .Lret0:
> mov result, #0
> ret
> -ENDPIPROC(memcmp)
> +SYM_FUNC_END_PI(memcmp)
> EXPORT_SYMBOL_NOKASAN(memcmp)
> diff --git a/arch/arm64/lib/memcpy.S b/arch/arm64/lib/memcpy.S
> index d79f48994dbb..9f382adfa88a 100644
> --- a/arch/arm64/lib/memcpy.S
> +++ b/arch/arm64/lib/memcpy.S
> @@ -57,11 +57,11 @@
> .endm
>
> .weak memcpy
Hmm, any idea why we use .weak explicitly here? Maybe better off using
the proper macros now? (same applies to many of the other lib/ functions
you're touching)
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-01-07 14:44 UTC|newest]
Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-01-06 19:58 [PATCH 0/3] arm64: Conversions to modern assembly annotations Mark Brown
2020-01-06 19:58 ` [PATCH 1/3] arm64: asm: Add new-style position independent function annotations Mark Brown
2020-01-07 14:44 ` Will Deacon
2020-01-07 16:45 ` Mark Brown
2020-01-06 19:58 ` [PATCH 2/3] arm64: lib: Use modern annotations for assembly functions Mark Brown
2020-01-07 14:44 ` Will Deacon [this message]
2020-01-07 17:47 ` Mark Brown
2020-01-08 12:29 ` Will Deacon
2020-01-10 16:56 ` Jiri Slaby
2020-01-15 18:43 ` Will Deacon
2020-01-06 19:58 ` [PATCH 3/3] arm64: mm: " Mark Brown
2020-01-07 14:43 ` Will Deacon
2020-01-07 16:42 ` Mark Brown
2020-01-08 12:17 ` Will Deacon
2020-01-08 13:50 ` Mark Brown
2020-01-08 14:56 ` Will Deacon
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=20200107144445.GC29001@willie-the-truck \
--to=will@kernel.org \
--cc=broonie@kernel.org \
--cc=catalin.marinas@arm.com \
--cc=jslaby@suse.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