[PATCH v2 0/8] arm64: String function updates

[PATCH v2 0/8] arm64: String function updates

[Robin Murphy]

Hi all,

Here's a proper update with the relicensing clarified and Mark's other
review comments addressed on patch #8.

v1: https://lore.kernel.org/linux-arm-kernel/cover.1620738177.git.robin.murphy@arm.com/

Thanks,
Robin.


Robin Murphy (4):
  arm64: Add assembly annotations for weak-PI-alias madness
  arm64: Import latest memcpy()/memmove() implementation
  arm64: Better optimised memchr()
  arm64: Rewrite __arch_clear_user()

Sam Tebbs (4):
  arm64: Import latest version of Cortex Strings' memcmp
  arm64: Import latest version of Cortex Strings' strcmp
  arm64: Import updated version of Cortex Strings' strlen
  arm64: Import latest version of Cortex Strings' strncmp

 arch/arm64/include/asm/linkage.h |   8 +
 arch/arm64/lib/Makefile          |   2 +-
 arch/arm64/lib/clear_user.S      |  47 ++--
 arch/arm64/lib/memchr.S          |  65 ++++-
 arch/arm64/lib/memcmp.S          | 330 ++++++++---------------
 arch/arm64/lib/memcpy.S          | 272 ++++++++++++++++---
 arch/arm64/lib/memmove.S         | 189 --------------
 arch/arm64/lib/strcmp.S          | 295 +++++++++------------
 arch/arm64/lib/strlen.S          | 262 +++++++++++++------
 arch/arm64/lib/strncmp.S         | 436 ++++++++++++++-----------------
 10 files changed, 927 insertions(+), 979 deletions(-)
 delete mode 100644 arch/arm64/lib/memmove.S

-- 
2.21.0.dirty


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

[PATCH v2 1/8] arm64: Import latest version of Cortex Strings' memcmp

[Robin Murphy]

From: Sam Tebbs <sam.tebbs@arm.com>

Import the latest version of the former Cortex Strings - now
Arm Optimized Routines - memcmp function based on the upstream
code of string/aarch64/memcmp.S at commit e823e3a from
https://github.com/ARM-software/optimized-routines

Note that for simplicity Arm have chosen to contribute this code
to Linux under GPLv2 rather than the original MIT license.

Signed-off-by: Sam Tebbs <sam.tebbs@arm.com>
[ rm: update attribution and commit message ]
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
 arch/arm64/lib/memcmp.S | 330 ++++++++++++++--------------------------
 1 file changed, 111 insertions(+), 219 deletions(-)

diff --git a/arch/arm64/lib/memcmp.S b/arch/arm64/lib/memcmp.S
index c0671e793ea9..498f0d9941d9 100644
--- a/arch/arm64/lib/memcmp.S
+++ b/arch/arm64/lib/memcmp.S
@@ -1,247 +1,139 @@
 /* SPDX-License-Identifier: GPL-2.0-only */
 /*
- * Copyright (C) 2013 ARM Ltd.
- * Copyright (C) 2013 Linaro.
+ * Copyright (c) 2013-2020, Arm Limited.
  *
- * This code is based on glibc cortex strings work originally authored by Linaro
- * be found @
- *
- * http://bazaar.launchpad.net/~linaro-toolchain-dev/cortex-strings/trunk/
- * files/head:/src/aarch64/
+ * Adapted from the original at:
+ * https://github.com/ARM-software/optimized-routines/blob/master/string/aarch64/memcmp.S
  */
 
 #include <linux/linkage.h>
 #include <asm/assembler.h>
 
-/*
-* compare memory areas(when two memory areas' offset are different,
-* alignment handled by the hardware)
-*
-* Parameters:
-*  x0 - const memory area 1 pointer
-*  x1 - const memory area 2 pointer
-*  x2 - the maximal compare byte length
-* Returns:
-*  x0 - a compare result, maybe less than, equal to, or greater than ZERO
-*/
+/* Assumptions:
+ *
+ * ARMv8-a, AArch64, unaligned accesses.
+ */
+
+#define L(label) .L ## label
 
 /* Parameters and result.  */
-src1		.req	x0
-src2		.req	x1
-limit		.req	x2
-result		.req	x0
+#define src1		x0
+#define src2		x1
+#define limit		x2
+#define result		w0
 
 /* Internal variables.  */
-data1		.req	x3
-data1w		.req	w3
-data2		.req	x4
-data2w		.req	w4
-has_nul		.req	x5
-diff		.req	x6
-endloop		.req	x7
-tmp1		.req	x8
-tmp2		.req	x9
-tmp3		.req	x10
-pos		.req	x11
-limit_wd	.req	x12
-mask		.req	x13
+#define data1		x3
+#define data1w		w3
+#define data1h		x4
+#define data2		x5
+#define data2w		w5
+#define data2h		x6
+#define tmp1		x7
+#define tmp2		x8
 
 SYM_FUNC_START_WEAK_PI(memcmp)
-	cbz	limit, .Lret0
-	eor	tmp1, src1, src2
-	tst	tmp1, #7
-	b.ne	.Lmisaligned8
-	ands	tmp1, src1, #7
-	b.ne	.Lmutual_align
-	sub	limit_wd, limit, #1 /* limit != 0, so no underflow.  */
-	lsr	limit_wd, limit_wd, #3 /* Convert to Dwords.  */
-	/*
-	* The input source addresses are at alignment boundary.
-	* Directly compare eight bytes each time.
-	*/
-.Lloop_aligned:
-	ldr	data1, [src1], #8
-	ldr	data2, [src2], #8
-.Lstart_realigned:
-	subs	limit_wd, limit_wd, #1
-	eor	diff, data1, data2	/* Non-zero if differences found.  */
-	csinv	endloop, diff, xzr, cs	/* Last Dword or differences.  */
-	cbz	endloop, .Lloop_aligned
+	subs	limit, limit, 8
+	b.lo	L(less8)
 
-	/* Not reached the limit, must have found a diff.  */
-	tbz	limit_wd, #63, .Lnot_limit
+	ldr	data1, [src1], 8
+	ldr	data2, [src2], 8
+	cmp	data1, data2
+	b.ne	L(return)
 
-	/* Limit % 8 == 0 => the diff is in the last 8 bytes. */
-	ands	limit, limit, #7
-	b.eq	.Lnot_limit
-	/*
-	* The remained bytes less than 8. It is needed to extract valid data
-	* from last eight bytes of the intended memory range.
-	*/
-	lsl	limit, limit, #3	/* bytes-> bits.  */
-	mov	mask, #~0
-CPU_BE( lsr	mask, mask, limit )
-CPU_LE( lsl	mask, mask, limit )
-	bic	data1, data1, mask
-	bic	data2, data2, mask
+	subs	limit, limit, 8
+	b.gt	L(more16)
 
-	orr	diff, diff, mask
-	b	.Lnot_limit
+	ldr	data1, [src1, limit]
+	ldr	data2, [src2, limit]
+	b	L(return)
 
-.Lmutual_align:
-	/*
-	* Sources are mutually aligned, but are not currently at an
-	* alignment boundary. Round down the addresses and then mask off
-	* the bytes that precede the start point.
-	*/
-	bic	src1, src1, #7
-	bic	src2, src2, #7
-	ldr	data1, [src1], #8
-	ldr	data2, [src2], #8
-	/*
-	* We can not add limit with alignment offset(tmp1) here. Since the
-	* addition probably make the limit overflown.
-	*/
-	sub	limit_wd, limit, #1/*limit != 0, so no underflow.*/
-	and	tmp3, limit_wd, #7
-	lsr	limit_wd, limit_wd, #3
-	add	tmp3, tmp3, tmp1
-	add	limit_wd, limit_wd, tmp3, lsr #3
-	add	limit, limit, tmp1/* Adjust the limit for the extra.  */
+L(more16):
+	ldr	data1, [src1], 8
+	ldr	data2, [src2], 8
+	cmp	data1, data2
+	bne	L(return)
 
-	lsl	tmp1, tmp1, #3/* Bytes beyond alignment -> bits.*/
-	neg	tmp1, tmp1/* Bits to alignment -64.  */
-	mov	tmp2, #~0
-	/*mask off the non-intended bytes before the start address.*/
-CPU_BE( lsl	tmp2, tmp2, tmp1 )/*Big-endian.Early bytes are at MSB*/
-	/* Little-endian.  Early bytes are at LSB.  */
-CPU_LE( lsr	tmp2, tmp2, tmp1 )
+	/* Jump directly to comparing the last 16 bytes for 32 byte (or less)
+	   strings.  */
+	subs	limit, limit, 16
+	b.ls	L(last_bytes)
 
-	orr	data1, data1, tmp2
-	orr	data2, data2, tmp2
-	b	.Lstart_realigned
+	/* We overlap loads between 0-32 bytes at either side of SRC1 when we
+	   try to align, so limit it only to strings larger than 128 bytes.  */
+	cmp	limit, 96
+	b.ls	L(loop16)
 
-	/*src1 and src2 have different alignment offset.*/
-.Lmisaligned8:
-	cmp	limit, #8
-	b.lo	.Ltiny8proc /*limit < 8: compare byte by byte*/
+	/* Align src1 and adjust src2 with bytes not yet done.  */
+	and	tmp1, src1, 15
+	add	limit, limit, tmp1
+	sub	src1, src1, tmp1
+	sub	src2, src2, tmp1
 
-	and	tmp1, src1, #7
-	neg	tmp1, tmp1
-	add	tmp1, tmp1, #8/*valid length in the first 8 bytes of src1*/
-	and	tmp2, src2, #7
-	neg	tmp2, tmp2
-	add	tmp2, tmp2, #8/*valid length in the first 8 bytes of src2*/
-	subs	tmp3, tmp1, tmp2
-	csel	pos, tmp1, tmp2, hi /*Choose the maximum.*/
+	/* Loop performing 16 bytes per iteration using aligned src1.
+	   Limit is pre-decremented by 16 and must be larger than zero.
+	   Exit if <= 16 bytes left to do or if the data is not equal.  */
+	.p2align 4
+L(loop16):
+	ldp	data1, data1h, [src1], 16
+	ldp	data2, data2h, [src2], 16
+	subs	limit, limit, 16
+	ccmp	data1, data2, 0, hi
+	ccmp	data1h, data2h, 0, eq
+	b.eq	L(loop16)
 
-	sub	limit, limit, pos
-	/*compare the proceeding bytes in the first 8 byte segment.*/
-.Ltinycmp:
-	ldrb	data1w, [src1], #1
-	ldrb	data2w, [src2], #1
-	subs	pos, pos, #1
-	ccmp	data1w, data2w, #0, ne  /* NZCV = 0b0000.  */
-	b.eq	.Ltinycmp
-	cbnz	pos, 1f /*diff occurred before the last byte.*/
+	cmp	data1, data2
+	bne	L(return)
+	mov	data1, data1h
+	mov	data2, data2h
+	cmp	data1, data2
+	bne	L(return)
+
+	/* Compare last 1-16 bytes using unaligned access.  */
+L(last_bytes):
+	add	src1, src1, limit
+	add	src2, src2, limit
+	ldp	data1, data1h, [src1]
+	ldp	data2, data2h, [src2]
+	cmp	data1, data2
+	bne	L(return)
+	mov	data1, data1h
+	mov	data2, data2h
+	cmp	data1, data2
+
+	/* Compare data bytes and set return value to 0, -1 or 1.  */
+L(return):
+#ifndef __AARCH64EB__
+	rev	data1, data1
+	rev	data2, data2
+#endif
+	cmp	data1, data2
+L(ret_eq):
+	cset	result, ne
+	cneg	result, result, lo
+	ret
+
+	.p2align 4
+	/* Compare up to 8 bytes.  Limit is [-8..-1].  */
+L(less8):
+	adds	limit, limit, 4
+	b.lo	L(less4)
+	ldr	data1w, [src1], 4
+	ldr	data2w, [src2], 4
 	cmp	data1w, data2w
-	b.eq	.Lstart_align
-1:
-	sub	result, data1, data2
+	b.ne	L(return)
+	sub	limit, limit, 4
+L(less4):
+	adds	limit, limit, 4
+	beq	L(ret_eq)
+L(byte_loop):
+	ldrb	data1w, [src1], 1
+	ldrb	data2w, [src2], 1
+	subs	limit, limit, 1
+	ccmp	data1w, data2w, 0, ne	/* NZCV = 0b0000.  */
+	b.eq	L(byte_loop)
+	sub	result, data1w, data2w
 	ret
 
-.Lstart_align:
-	lsr	limit_wd, limit, #3
-	cbz	limit_wd, .Lremain8
-
-	ands	xzr, src1, #7
-	b.eq	.Lrecal_offset
-	/*process more leading bytes to make src1 aligned...*/
-	add	src1, src1, tmp3 /*backwards src1 to alignment boundary*/
-	add	src2, src2, tmp3
-	sub	limit, limit, tmp3
-	lsr	limit_wd, limit, #3
-	cbz	limit_wd, .Lremain8
-	/*load 8 bytes from aligned SRC1..*/
-	ldr	data1, [src1], #8
-	ldr	data2, [src2], #8
-
-	subs	limit_wd, limit_wd, #1
-	eor	diff, data1, data2  /*Non-zero if differences found.*/
-	csinv	endloop, diff, xzr, ne
-	cbnz	endloop, .Lunequal_proc
-	/*How far is the current SRC2 from the alignment boundary...*/
-	and	tmp3, tmp3, #7
-
-.Lrecal_offset:/*src1 is aligned now..*/
-	neg	pos, tmp3
-.Lloopcmp_proc:
-	/*
-	* Divide the eight bytes into two parts. First,backwards the src2
-	* to an alignment boundary,load eight bytes and compare from
-	* the SRC2 alignment boundary. If all 8 bytes are equal,then start
-	* the second part's comparison. Otherwise finish the comparison.
-	* This special handle can garantee all the accesses are in the
-	* thread/task space in avoid to overrange access.
-	*/
-	ldr	data1, [src1,pos]
-	ldr	data2, [src2,pos]
-	eor	diff, data1, data2  /* Non-zero if differences found.  */
-	cbnz	diff, .Lnot_limit
-
-	/*The second part process*/
-	ldr	data1, [src1], #8
-	ldr	data2, [src2], #8
-	eor	diff, data1, data2  /* Non-zero if differences found.  */
-	subs	limit_wd, limit_wd, #1
-	csinv	endloop, diff, xzr, ne/*if limit_wd is 0,will finish the cmp*/
-	cbz	endloop, .Lloopcmp_proc
-.Lunequal_proc:
-	cbz	diff, .Lremain8
-
-/* There is difference occurred in the latest comparison. */
-.Lnot_limit:
-/*
-* For little endian,reverse the low significant equal bits into MSB,then
-* following CLZ can find how many equal bits exist.
-*/
-CPU_LE( rev	diff, diff )
-CPU_LE( rev	data1, data1 )
-CPU_LE( rev	data2, data2 )
-
-	/*
-	* The MS-non-zero bit of DIFF marks either the first bit
-	* that is different, or the end of the significant data.
-	* Shifting left now will bring the critical information into the
-	* top bits.
-	*/
-	clz	pos, diff
-	lsl	data1, data1, pos
-	lsl	data2, data2, pos
-	/*
-	* We need to zero-extend (char is unsigned) the value and then
-	* perform a signed subtraction.
-	*/
-	lsr	data1, data1, #56
-	sub	result, data1, data2, lsr #56
-	ret
-
-.Lremain8:
-	/* Limit % 8 == 0 =>. all data are equal.*/
-	ands	limit, limit, #7
-	b.eq	.Lret0
-
-.Ltiny8proc:
-	ldrb	data1w, [src1], #1
-	ldrb	data2w, [src2], #1
-	subs	limit, limit, #1
-
-	ccmp	data1w, data2w, #0, ne  /* NZCV = 0b0000. */
-	b.eq	.Ltiny8proc
-	sub	result, data1, data2
-	ret
-.Lret0:
-	mov	result, #0
-	ret
 SYM_FUNC_END_PI(memcmp)
 EXPORT_SYMBOL_NOKASAN(memcmp)
-- 
2.21.0.dirty


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

Re: [PATCH v2 1/8] arm64: Import latest version of Cortex Strings' memcmp

[Mark Rutland]

Hi Robin,

On Thu, May 27, 2021 at 04:34:41PM +0100, Robin Murphy wrote:
> From: Sam Tebbs <sam.tebbs@arm.com>
> 
> Import the latest version of the former Cortex Strings - now
> Arm Optimized Routines - memcmp function based on the upstream
> code of string/aarch64/memcmp.S at commit e823e3a from
> https://github.com/ARM-software/optimized-routines
> 
> Note that for simplicity Arm have chosen to contribute this code
> to Linux under GPLv2 rather than the original MIT license.

Thanks for adding this! It would be nice if we could make this slightly
more explicit, e.g.

| Arm hold the copyright on this code, and have chosen to contribute it
| to linux under GPL v2 rather than the original MIT license.

... but I don't think that requires a respin.

> 
> Signed-off-by: Sam Tebbs <sam.tebbs@arm.com>
> [ rm: update attribution and commit message ]
> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
> ---
>  arch/arm64/lib/memcmp.S | 330 ++++++++++++++--------------------------
>  1 file changed, 111 insertions(+), 219 deletions(-)
> 
> diff --git a/arch/arm64/lib/memcmp.S b/arch/arm64/lib/memcmp.S
> index c0671e793ea9..498f0d9941d9 100644
> --- a/arch/arm64/lib/memcmp.S
> +++ b/arch/arm64/lib/memcmp.S
> @@ -1,247 +1,139 @@
>  /* SPDX-License-Identifier: GPL-2.0-only */
>  /*
> - * Copyright (C) 2013 ARM Ltd.
> - * Copyright (C) 2013 Linaro.
> + * Copyright (c) 2013-2020, Arm Limited.

2021 too, presumably. ;)

>   *
> - * This code is based on glibc cortex strings work originally authored by Linaro
> - * be found @
> - *
> - * http://bazaar.launchpad.net/~linaro-toolchain-dev/cortex-strings/trunk/
> - * files/head:/src/aarch64/
> + * Adapted from the original at:
> + * https://github.com/ARM-software/optimized-routines/blob/master/string/aarch64/memcmp.S
>   */

Could we please note the commit ID in the comment, too? e.g. we can
place that in the URL:

https://github.com/ARM-software/optimized-routines/blob/e823e3abf5f89ecb/string/aarch64/memcmp.S

I've checked that the below matches the code in the file referenced
above, and AFAICT, the only change I see in the body of the function is
dropping PTR_ARG() and SIZE_ARG(), which are not necessary as the kernel
itself is not built with ILP32. The local definition of L() also seems
fine to me.

On the assumption that the code was correct to begin with, this looks
good to me. With the minor fixups above:

Acked-by: Mark Rutland <mark.rutland@arm.com>

Thanks,
Mark.

>  
>  #include <linux/linkage.h>
>  #include <asm/assembler.h>
>  
> -/*
> -* compare memory areas(when two memory areas' offset are different,
> -* alignment handled by the hardware)
> -*
> -* Parameters:
> -*  x0 - const memory area 1 pointer
> -*  x1 - const memory area 2 pointer
> -*  x2 - the maximal compare byte length
> -* Returns:
> -*  x0 - a compare result, maybe less than, equal to, or greater than ZERO
> -*/
> +/* Assumptions:
> + *
> + * ARMv8-a, AArch64, unaligned accesses.
> + */
> +
> +#define L(label) .L ## label
>  
>  /* Parameters and result.  */
> -src1		.req	x0
> -src2		.req	x1
> -limit		.req	x2
> -result		.req	x0
> +#define src1		x0
> +#define src2		x1
> +#define limit		x2
> +#define result		w0
>  
>  /* Internal variables.  */
> -data1		.req	x3
> -data1w		.req	w3
> -data2		.req	x4
> -data2w		.req	w4
> -has_nul		.req	x5
> -diff		.req	x6
> -endloop		.req	x7
> -tmp1		.req	x8
> -tmp2		.req	x9
> -tmp3		.req	x10
> -pos		.req	x11
> -limit_wd	.req	x12
> -mask		.req	x13
> +#define data1		x3
> +#define data1w		w3
> +#define data1h		x4
> +#define data2		x5
> +#define data2w		w5
> +#define data2h		x6
> +#define tmp1		x7
> +#define tmp2		x8
>  
>  SYM_FUNC_START_WEAK_PI(memcmp)
> -	cbz	limit, .Lret0
> -	eor	tmp1, src1, src2
> -	tst	tmp1, #7
> -	b.ne	.Lmisaligned8
> -	ands	tmp1, src1, #7
> -	b.ne	.Lmutual_align
> -	sub	limit_wd, limit, #1 /* limit != 0, so no underflow.  */
> -	lsr	limit_wd, limit_wd, #3 /* Convert to Dwords.  */
> -	/*
> -	* The input source addresses are at alignment boundary.
> -	* Directly compare eight bytes each time.
> -	*/
> -.Lloop_aligned:
> -	ldr	data1, [src1], #8
> -	ldr	data2, [src2], #8
> -.Lstart_realigned:
> -	subs	limit_wd, limit_wd, #1
> -	eor	diff, data1, data2	/* Non-zero if differences found.  */
> -	csinv	endloop, diff, xzr, cs	/* Last Dword or differences.  */
> -	cbz	endloop, .Lloop_aligned
> +	subs	limit, limit, 8
> +	b.lo	L(less8)
>  
> -	/* Not reached the limit, must have found a diff.  */
> -	tbz	limit_wd, #63, .Lnot_limit
> +	ldr	data1, [src1], 8
> +	ldr	data2, [src2], 8
> +	cmp	data1, data2
> +	b.ne	L(return)
>  
> -	/* Limit % 8 == 0 => the diff is in the last 8 bytes. */
> -	ands	limit, limit, #7
> -	b.eq	.Lnot_limit
> -	/*
> -	* The remained bytes less than 8. It is needed to extract valid data
> -	* from last eight bytes of the intended memory range.
> -	*/
> -	lsl	limit, limit, #3	/* bytes-> bits.  */
> -	mov	mask, #~0
> -CPU_BE( lsr	mask, mask, limit )
> -CPU_LE( lsl	mask, mask, limit )
> -	bic	data1, data1, mask
> -	bic	data2, data2, mask
> +	subs	limit, limit, 8
> +	b.gt	L(more16)
>  
> -	orr	diff, diff, mask
> -	b	.Lnot_limit
> +	ldr	data1, [src1, limit]
> +	ldr	data2, [src2, limit]
> +	b	L(return)
>  
> -.Lmutual_align:
> -	/*
> -	* Sources are mutually aligned, but are not currently at an
> -	* alignment boundary. Round down the addresses and then mask off
> -	* the bytes that precede the start point.
> -	*/
> -	bic	src1, src1, #7
> -	bic	src2, src2, #7
> -	ldr	data1, [src1], #8
> -	ldr	data2, [src2], #8
> -	/*
> -	* We can not add limit with alignment offset(tmp1) here. Since the
> -	* addition probably make the limit overflown.
> -	*/
> -	sub	limit_wd, limit, #1/*limit != 0, so no underflow.*/
> -	and	tmp3, limit_wd, #7
> -	lsr	limit_wd, limit_wd, #3
> -	add	tmp3, tmp3, tmp1
> -	add	limit_wd, limit_wd, tmp3, lsr #3
> -	add	limit, limit, tmp1/* Adjust the limit for the extra.  */
> +L(more16):
> +	ldr	data1, [src1], 8
> +	ldr	data2, [src2], 8
> +	cmp	data1, data2
> +	bne	L(return)
>  
> -	lsl	tmp1, tmp1, #3/* Bytes beyond alignment -> bits.*/
> -	neg	tmp1, tmp1/* Bits to alignment -64.  */
> -	mov	tmp2, #~0
> -	/*mask off the non-intended bytes before the start address.*/
> -CPU_BE( lsl	tmp2, tmp2, tmp1 )/*Big-endian.Early bytes are at MSB*/
> -	/* Little-endian.  Early bytes are at LSB.  */
> -CPU_LE( lsr	tmp2, tmp2, tmp1 )
> +	/* Jump directly to comparing the last 16 bytes for 32 byte (or less)
> +	   strings.  */
> +	subs	limit, limit, 16
> +	b.ls	L(last_bytes)
>  
> -	orr	data1, data1, tmp2
> -	orr	data2, data2, tmp2
> -	b	.Lstart_realigned
> +	/* We overlap loads between 0-32 bytes at either side of SRC1 when we
> +	   try to align, so limit it only to strings larger than 128 bytes.  */
> +	cmp	limit, 96
> +	b.ls	L(loop16)
>  
> -	/*src1 and src2 have different alignment offset.*/
> -.Lmisaligned8:
> -	cmp	limit, #8
> -	b.lo	.Ltiny8proc /*limit < 8: compare byte by byte*/
> +	/* Align src1 and adjust src2 with bytes not yet done.  */
> +	and	tmp1, src1, 15
> +	add	limit, limit, tmp1
> +	sub	src1, src1, tmp1
> +	sub	src2, src2, tmp1
>  
> -	and	tmp1, src1, #7
> -	neg	tmp1, tmp1
> -	add	tmp1, tmp1, #8/*valid length in the first 8 bytes of src1*/
> -	and	tmp2, src2, #7
> -	neg	tmp2, tmp2
> -	add	tmp2, tmp2, #8/*valid length in the first 8 bytes of src2*/
> -	subs	tmp3, tmp1, tmp2
> -	csel	pos, tmp1, tmp2, hi /*Choose the maximum.*/
> +	/* Loop performing 16 bytes per iteration using aligned src1.
> +	   Limit is pre-decremented by 16 and must be larger than zero.
> +	   Exit if <= 16 bytes left to do or if the data is not equal.  */
> +	.p2align 4
> +L(loop16):
> +	ldp	data1, data1h, [src1], 16
> +	ldp	data2, data2h, [src2], 16
> +	subs	limit, limit, 16
> +	ccmp	data1, data2, 0, hi
> +	ccmp	data1h, data2h, 0, eq
> +	b.eq	L(loop16)
>  
> -	sub	limit, limit, pos
> -	/*compare the proceeding bytes in the first 8 byte segment.*/
> -.Ltinycmp:
> -	ldrb	data1w, [src1], #1
> -	ldrb	data2w, [src2], #1
> -	subs	pos, pos, #1
> -	ccmp	data1w, data2w, #0, ne  /* NZCV = 0b0000.  */
> -	b.eq	.Ltinycmp
> -	cbnz	pos, 1f /*diff occurred before the last byte.*/
> +	cmp	data1, data2
> +	bne	L(return)
> +	mov	data1, data1h
> +	mov	data2, data2h
> +	cmp	data1, data2
> +	bne	L(return)
> +
> +	/* Compare last 1-16 bytes using unaligned access.  */
> +L(last_bytes):
> +	add	src1, src1, limit
> +	add	src2, src2, limit
> +	ldp	data1, data1h, [src1]
> +	ldp	data2, data2h, [src2]
> +	cmp	data1, data2
> +	bne	L(return)
> +	mov	data1, data1h
> +	mov	data2, data2h
> +	cmp	data1, data2
> +
> +	/* Compare data bytes and set return value to 0, -1 or 1.  */
> +L(return):
> +#ifndef __AARCH64EB__
> +	rev	data1, data1
> +	rev	data2, data2
> +#endif
> +	cmp	data1, data2
> +L(ret_eq):
> +	cset	result, ne
> +	cneg	result, result, lo
> +	ret
> +
> +	.p2align 4
> +	/* Compare up to 8 bytes.  Limit is [-8..-1].  */
> +L(less8):
> +	adds	limit, limit, 4
> +	b.lo	L(less4)
> +	ldr	data1w, [src1], 4
> +	ldr	data2w, [src2], 4
>  	cmp	data1w, data2w
> -	b.eq	.Lstart_align
> -1:
> -	sub	result, data1, data2
> +	b.ne	L(return)
> +	sub	limit, limit, 4
> +L(less4):
> +	adds	limit, limit, 4
> +	beq	L(ret_eq)
> +L(byte_loop):
> +	ldrb	data1w, [src1], 1
> +	ldrb	data2w, [src2], 1
> +	subs	limit, limit, 1
> +	ccmp	data1w, data2w, 0, ne	/* NZCV = 0b0000.  */
> +	b.eq	L(byte_loop)
> +	sub	result, data1w, data2w
>  	ret
>  
> -.Lstart_align:
> -	lsr	limit_wd, limit, #3
> -	cbz	limit_wd, .Lremain8
> -
> -	ands	xzr, src1, #7
> -	b.eq	.Lrecal_offset
> -	/*process more leading bytes to make src1 aligned...*/
> -	add	src1, src1, tmp3 /*backwards src1 to alignment boundary*/
> -	add	src2, src2, tmp3
> -	sub	limit, limit, tmp3
> -	lsr	limit_wd, limit, #3
> -	cbz	limit_wd, .Lremain8
> -	/*load 8 bytes from aligned SRC1..*/
> -	ldr	data1, [src1], #8
> -	ldr	data2, [src2], #8
> -
> -	subs	limit_wd, limit_wd, #1
> -	eor	diff, data1, data2  /*Non-zero if differences found.*/
> -	csinv	endloop, diff, xzr, ne
> -	cbnz	endloop, .Lunequal_proc
> -	/*How far is the current SRC2 from the alignment boundary...*/
> -	and	tmp3, tmp3, #7
> -
> -.Lrecal_offset:/*src1 is aligned now..*/
> -	neg	pos, tmp3
> -.Lloopcmp_proc:
> -	/*
> -	* Divide the eight bytes into two parts. First,backwards the src2
> -	* to an alignment boundary,load eight bytes and compare from
> -	* the SRC2 alignment boundary. If all 8 bytes are equal,then start
> -	* the second part's comparison. Otherwise finish the comparison.
> -	* This special handle can garantee all the accesses are in the
> -	* thread/task space in avoid to overrange access.
> -	*/
> -	ldr	data1, [src1,pos]
> -	ldr	data2, [src2,pos]
> -	eor	diff, data1, data2  /* Non-zero if differences found.  */
> -	cbnz	diff, .Lnot_limit
> -
> -	/*The second part process*/
> -	ldr	data1, [src1], #8
> -	ldr	data2, [src2], #8
> -	eor	diff, data1, data2  /* Non-zero if differences found.  */
> -	subs	limit_wd, limit_wd, #1
> -	csinv	endloop, diff, xzr, ne/*if limit_wd is 0,will finish the cmp*/
> -	cbz	endloop, .Lloopcmp_proc
> -.Lunequal_proc:
> -	cbz	diff, .Lremain8
> -
> -/* There is difference occurred in the latest comparison. */
> -.Lnot_limit:
> -/*
> -* For little endian,reverse the low significant equal bits into MSB,then
> -* following CLZ can find how many equal bits exist.
> -*/
> -CPU_LE( rev	diff, diff )
> -CPU_LE( rev	data1, data1 )
> -CPU_LE( rev	data2, data2 )
> -
> -	/*
> -	* The MS-non-zero bit of DIFF marks either the first bit
> -	* that is different, or the end of the significant data.
> -	* Shifting left now will bring the critical information into the
> -	* top bits.
> -	*/
> -	clz	pos, diff
> -	lsl	data1, data1, pos
> -	lsl	data2, data2, pos
> -	/*
> -	* We need to zero-extend (char is unsigned) the value and then
> -	* perform a signed subtraction.
> -	*/
> -	lsr	data1, data1, #56
> -	sub	result, data1, data2, lsr #56
> -	ret
> -
> -.Lremain8:
> -	/* Limit % 8 == 0 =>. all data are equal.*/
> -	ands	limit, limit, #7
> -	b.eq	.Lret0
> -
> -.Ltiny8proc:
> -	ldrb	data1w, [src1], #1
> -	ldrb	data2w, [src2], #1
> -	subs	limit, limit, #1
> -
> -	ccmp	data1w, data2w, #0, ne  /* NZCV = 0b0000. */
> -	b.eq	.Ltiny8proc
> -	sub	result, data1, data2
> -	ret
> -.Lret0:
> -	mov	result, #0
> -	ret
>  SYM_FUNC_END_PI(memcmp)
>  EXPORT_SYMBOL_NOKASAN(memcmp)
> -- 
> 2.21.0.dirty
> 

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

Re: [PATCH v2 1/8] arm64: Import latest version of Cortex Strings' memcmp

[Will Deacon]

On Thu, May 27, 2021 at 05:52:14PM +0100, Mark Rutland wrote:
> On Thu, May 27, 2021 at 04:34:41PM +0100, Robin Murphy wrote:
> > From: Sam Tebbs <sam.tebbs@arm.com>
> > 
> > Import the latest version of the former Cortex Strings - now
> > Arm Optimized Routines - memcmp function based on the upstream
> > code of string/aarch64/memcmp.S at commit e823e3a from
> > https://github.com/ARM-software/optimized-routines
> > 
> > Note that for simplicity Arm have chosen to contribute this code
> > to Linux under GPLv2 rather than the original MIT license.
> 
> Thanks for adding this! It would be nice if we could make this slightly
> more explicit, e.g.
> 
> | Arm hold the copyright on this code, and have chosen to contribute it
> | to linux under GPL v2 rather than the original MIT license.
> 
> ... but I don't think that requires a respin.
> 
> > 
> > Signed-off-by: Sam Tebbs <sam.tebbs@arm.com>
> > [ rm: update attribution and commit message ]
> > Signed-off-by: Robin Murphy <robin.murphy@arm.com>
> > ---
> >  arch/arm64/lib/memcmp.S | 330 ++++++++++++++--------------------------
> >  1 file changed, 111 insertions(+), 219 deletions(-)
> > 
> > diff --git a/arch/arm64/lib/memcmp.S b/arch/arm64/lib/memcmp.S
> > index c0671e793ea9..498f0d9941d9 100644
> > --- a/arch/arm64/lib/memcmp.S
> > +++ b/arch/arm64/lib/memcmp.S
> > @@ -1,247 +1,139 @@
> >  /* SPDX-License-Identifier: GPL-2.0-only */
> >  /*
> > - * Copyright (C) 2013 ARM Ltd.
> > - * Copyright (C) 2013 Linaro.
> > + * Copyright (c) 2013-2020, Arm Limited.
> 
> 2021 too, presumably. ;)
> 
> >   *
> > - * This code is based on glibc cortex strings work originally authored by Linaro
> > - * be found @
> > - *
> > - * http://bazaar.launchpad.net/~linaro-toolchain-dev/cortex-strings/trunk/
> > - * files/head:/src/aarch64/
> > + * Adapted from the original at:
> > + * https://github.com/ARM-software/optimized-routines/blob/master/string/aarch64/memcmp.S
> >   */
> 
> Could we please note the commit ID in the comment, too? e.g. we can
> place that in the URL:
> 
> https://github.com/ARM-software/optimized-routines/blob/e823e3abf5f89ecb/string/aarch64/memcmp.S
> 
> I've checked that the below matches the code in the file referenced
> above, and AFAICT, the only change I see in the body of the function is
> dropping PTR_ARG() and SIZE_ARG(), which are not necessary as the kernel
> itself is not built with ILP32. The local definition of L() also seems
> fine to me.
> 
> On the assumption that the code was correct to begin with, this looks
> good to me. With the minor fixups above:
> 
> Acked-by: Mark Rutland <mark.rutland@arm.com>

I've applied this as-is with your Ack, but I haven't yet put it into
linux-next. Please can you (or Robin) send patches on top to address the
above?

Sorry for rushing a bit, but I'd like to get CKI chewing on this ASAP
and didn't want to wait for a respin if it's just cosmetic stuff pending.

Cheers,

Will

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

[PATCH v2 2/8] arm64: Import latest version of Cortex Strings' strcmp

[Robin Murphy]

From: Sam Tebbs <sam.tebbs@arm.com>

Import the latest version of the former Cortex Strings - now
Arm Optimized Routines - strcmp function based on the upstream
code of string/aarch64/strcmp.S at commit afd6244 from
https://github.com/ARM-software/optimized-routines

Note that for simplicity Arm have chosen to contribute this code
to Linux under GPLv2 rather than the original MIT license.

Signed-off-by: Sam Tebbs <sam.tebbs@arm.com>
[ rm: update attribution and commit message ]
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
 arch/arm64/lib/strcmp.S | 295 +++++++++++++++++-----------------------
 1 file changed, 124 insertions(+), 171 deletions(-)

diff --git a/arch/arm64/lib/strcmp.S b/arch/arm64/lib/strcmp.S
index 4e79566726c8..e82ccb6c2f93 100644
--- a/arch/arm64/lib/strcmp.S
+++ b/arch/arm64/lib/strcmp.S
@@ -1,84 +1,123 @@
 /* SPDX-License-Identifier: GPL-2.0-only */
 /*
- * Copyright (C) 2013 ARM Ltd.
- * Copyright (C) 2013 Linaro.
+ * Copyright (c) 2012-2020, Arm Limited.
  *
- * This code is based on glibc cortex strings work originally authored by Linaro
- * be found @
- *
- * http://bazaar.launchpad.net/~linaro-toolchain-dev/cortex-strings/trunk/
- * files/head:/src/aarch64/
+ * Adapted from the original at:
+ * https://github.com/ARM-software/optimized-routines/blob/master/string/aarch64/strcmp.S
  */
 
 #include <linux/linkage.h>
 #include <asm/assembler.h>
 
-/*
- * compare two strings
+/* Assumptions:
  *
- * Parameters:
- *	x0 - const string 1 pointer
- *    x1 - const string 2 pointer
- * Returns:
- * x0 - an integer less than, equal to, or greater than zero
- * if  s1  is  found, respectively, to be less than, to match,
- * or be greater than s2.
+ * ARMv8-a, AArch64
  */
 
+#define L(label) .L ## label
+
 #define REP8_01 0x0101010101010101
 #define REP8_7f 0x7f7f7f7f7f7f7f7f
 #define REP8_80 0x8080808080808080
 
 /* Parameters and result.  */
-src1		.req	x0
-src2		.req	x1
-result		.req	x0
+#define src1		x0
+#define src2		x1
+#define result		x0
 
 /* Internal variables.  */
-data1		.req	x2
-data1w		.req	w2
-data2		.req	x3
-data2w		.req	w3
-has_nul		.req	x4
-diff		.req	x5
-syndrome	.req	x6
-tmp1		.req	x7
-tmp2		.req	x8
-tmp3		.req	x9
-zeroones	.req	x10
-pos		.req	x11
+#define data1		x2
+#define data1w		w2
+#define data2		x3
+#define data2w		w3
+#define has_nul		x4
+#define diff		x5
+#define syndrome	x6
+#define tmp1		x7
+#define tmp2		x8
+#define tmp3		x9
+#define zeroones	x10
+#define pos		x11
 
+	/* Start of performance-critical section  -- one 64B cache line.  */
+	.align 6
 SYM_FUNC_START_WEAK_PI(strcmp)
 	eor	tmp1, src1, src2
 	mov	zeroones, #REP8_01
 	tst	tmp1, #7
-	b.ne	.Lmisaligned8
+	b.ne	L(misaligned8)
 	ands	tmp1, src1, #7
-	b.ne	.Lmutual_align
-
-	/*
-	* NUL detection works on the principle that (X - 1) & (~X) & 0x80
-	* (=> (X - 1) & ~(X | 0x7f)) is non-zero iff a byte is zero, and
-	* can be done in parallel across the entire word.
-	*/
-.Lloop_aligned:
+	b.ne	L(mutual_align)
+	/* NUL detection works on the principle that (X - 1) & (~X) & 0x80
+	   (=> (X - 1) & ~(X | 0x7f)) is non-zero iff a byte is zero, and
+	   can be done in parallel across the entire word.  */
+L(loop_aligned):
 	ldr	data1, [src1], #8
 	ldr	data2, [src2], #8
-.Lstart_realigned:
+L(start_realigned):
 	sub	tmp1, data1, zeroones
 	orr	tmp2, data1, #REP8_7f
 	eor	diff, data1, data2	/* Non-zero if differences found.  */
 	bic	has_nul, tmp1, tmp2	/* Non-zero if NUL terminator.  */
 	orr	syndrome, diff, has_nul
-	cbz	syndrome, .Lloop_aligned
-	b	.Lcal_cmpresult
+	cbz	syndrome, L(loop_aligned)
+	/* End of performance-critical section  -- one 64B cache line.  */
 
-.Lmutual_align:
-	/*
-	* Sources are mutually aligned, but are not currently at an
-	* alignment boundary.  Round down the addresses and then mask off
-	* the bytes that preceed the start point.
-	*/
+L(end):
+#ifndef	__AARCH64EB__
+	rev	syndrome, syndrome
+	rev	data1, data1
+	/* The MS-non-zero bit of the syndrome marks either the first bit
+	   that is different, or the top bit of the first zero byte.
+	   Shifting left now will bring the critical information into the
+	   top bits.  */
+	clz	pos, syndrome
+	rev	data2, data2
+	lsl	data1, data1, pos
+	lsl	data2, data2, pos
+	/* But we need to zero-extend (char is unsigned) the value and then
+	   perform a signed 32-bit subtraction.  */
+	lsr	data1, data1, #56
+	sub	result, data1, data2, lsr #56
+	ret
+#else
+	/* For big-endian we cannot use the trick with the syndrome value
+	   as carry-propagation can corrupt the upper bits if the trailing
+	   bytes in the string contain 0x01.  */
+	/* However, if there is no NUL byte in the dword, we can generate
+	   the result directly.  We can't just subtract the bytes as the
+	   MSB might be significant.  */
+	cbnz	has_nul, 1f
+	cmp	data1, data2
+	cset	result, ne
+	cneg	result, result, lo
+	ret
+1:
+	/* Re-compute the NUL-byte detection, using a byte-reversed value.  */
+	rev	tmp3, data1
+	sub	tmp1, tmp3, zeroones
+	orr	tmp2, tmp3, #REP8_7f
+	bic	has_nul, tmp1, tmp2
+	rev	has_nul, has_nul
+	orr	syndrome, diff, has_nul
+	clz	pos, syndrome
+	/* The MS-non-zero bit of the syndrome marks either the first bit
+	   that is different, or the top bit of the first zero byte.
+	   Shifting left now will bring the critical information into the
+	   top bits.  */
+	lsl	data1, data1, pos
+	lsl	data2, data2, pos
+	/* But we need to zero-extend (char is unsigned) the value and then
+	   perform a signed 32-bit subtraction.  */
+	lsr	data1, data1, #56
+	sub	result, data1, data2, lsr #56
+	ret
+#endif
+
+L(mutual_align):
+	/* Sources are mutually aligned, but are not currently at an
+	   alignment boundary.  Round down the addresses and then mask off
+	   the bytes that preceed the start point.  */
 	bic	src1, src1, #7
 	bic	src2, src2, #7
 	lsl	tmp1, tmp1, #3		/* Bytes beyond alignment -> bits.  */
@@ -86,138 +125,52 @@ SYM_FUNC_START_WEAK_PI(strcmp)
 	neg	tmp1, tmp1		/* Bits to alignment -64.  */
 	ldr	data2, [src2], #8
 	mov	tmp2, #~0
+#ifdef __AARCH64EB__
 	/* Big-endian.  Early bytes are at MSB.  */
-CPU_BE( lsl	tmp2, tmp2, tmp1 )	/* Shift (tmp1 & 63).  */
+	lsl	tmp2, tmp2, tmp1	/* Shift (tmp1 & 63).  */
+#else
 	/* Little-endian.  Early bytes are at LSB.  */
-CPU_LE( lsr	tmp2, tmp2, tmp1 )	/* Shift (tmp1 & 63).  */
-
+	lsr	tmp2, tmp2, tmp1	/* Shift (tmp1 & 63).  */
+#endif
 	orr	data1, data1, tmp2
 	orr	data2, data2, tmp2
-	b	.Lstart_realigned
+	b	L(start_realigned)
 
-.Lmisaligned8:
-	/*
-	* Get the align offset length to compare per byte first.
-	* After this process, one string's address will be aligned.
-	*/
-	and	tmp1, src1, #7
-	neg	tmp1, tmp1
-	add	tmp1, tmp1, #8
-	and	tmp2, src2, #7
-	neg	tmp2, tmp2
-	add	tmp2, tmp2, #8
-	subs	tmp3, tmp1, tmp2
-	csel	pos, tmp1, tmp2, hi /*Choose the maximum. */
-.Ltinycmp:
+L(misaligned8):
+	/* Align SRC1 to 8 bytes and then compare 8 bytes at a time, always
+	   checking to make sure that we don't access beyond page boundary in
+	   SRC2.  */
+	tst	src1, #7
+	b.eq	L(loop_misaligned)
+L(do_misaligned):
 	ldrb	data1w, [src1], #1
 	ldrb	data2w, [src2], #1
-	subs	pos, pos, #1
-	ccmp	data1w, #1, #0, ne  /* NZCV = 0b0000.  */
-	ccmp	data1w, data2w, #0, cs  /* NZCV = 0b0000.  */
-	b.eq	.Ltinycmp
-	cbnz	pos, 1f /*find the null or unequal...*/
 	cmp	data1w, #1
-	ccmp	data1w, data2w, #0, cs
-	b.eq	.Lstart_align /*the last bytes are equal....*/
-1:
+	ccmp	data1w, data2w, #0, cs	/* NZCV = 0b0000.  */
+	b.ne	L(done)
+	tst	src1, #7
+	b.ne	L(do_misaligned)
+
+L(loop_misaligned):
+	/* Test if we are within the last dword of the end of a 4K page.  If
+	   yes then jump back to the misaligned loop to copy a byte at a time.  */
+	and	tmp1, src2, #0xff8
+	eor	tmp1, tmp1, #0xff8
+	cbz	tmp1, L(do_misaligned)
+	ldr	data1, [src1], #8
+	ldr	data2, [src2], #8
+
+	sub	tmp1, data1, zeroones
+	orr	tmp2, data1, #REP8_7f
+	eor	diff, data1, data2	/* Non-zero if differences found.  */
+	bic	has_nul, tmp1, tmp2	/* Non-zero if NUL terminator.  */
+	orr	syndrome, diff, has_nul
+	cbz	syndrome, L(loop_misaligned)
+	b	L(end)
+
+L(done):
 	sub	result, data1, data2
 	ret
 
-.Lstart_align:
-	ands	xzr, src1, #7
-	b.eq	.Lrecal_offset
-	/*process more leading bytes to make str1 aligned...*/
-	add	src1, src1, tmp3
-	add	src2, src2, tmp3
-	/*load 8 bytes from aligned str1 and non-aligned str2..*/
-	ldr	data1, [src1], #8
-	ldr	data2, [src2], #8
-
-	sub	tmp1, data1, zeroones
-	orr	tmp2, data1, #REP8_7f
-	bic	has_nul, tmp1, tmp2
-	eor	diff, data1, data2 /* Non-zero if differences found.  */
-	orr	syndrome, diff, has_nul
-	cbnz	syndrome, .Lcal_cmpresult
-	/*How far is the current str2 from the alignment boundary...*/
-	and	tmp3, tmp3, #7
-.Lrecal_offset:
-	neg	pos, tmp3
-.Lloopcmp_proc:
-	/*
-	* Divide the eight bytes into two parts. First,backwards the src2
-	* to an alignment boundary,load eight bytes from the SRC2 alignment
-	* boundary,then compare with the relative bytes from SRC1.
-	* If all 8 bytes are equal,then start the second part's comparison.
-	* Otherwise finish the comparison.
-	* This special handle can garantee all the accesses are in the
-	* thread/task space in avoid to overrange access.
-	*/
-	ldr	data1, [src1,pos]
-	ldr	data2, [src2,pos]
-	sub	tmp1, data1, zeroones
-	orr	tmp2, data1, #REP8_7f
-	bic	has_nul, tmp1, tmp2
-	eor	diff, data1, data2  /* Non-zero if differences found.  */
-	orr	syndrome, diff, has_nul
-	cbnz	syndrome, .Lcal_cmpresult
-
-	/*The second part process*/
-	ldr	data1, [src1], #8
-	ldr	data2, [src2], #8
-	sub	tmp1, data1, zeroones
-	orr	tmp2, data1, #REP8_7f
-	bic	has_nul, tmp1, tmp2
-	eor	diff, data1, data2  /* Non-zero if differences found.  */
-	orr	syndrome, diff, has_nul
-	cbz	syndrome, .Lloopcmp_proc
-
-.Lcal_cmpresult:
-	/*
-	* reversed the byte-order as big-endian,then CLZ can find the most
-	* significant zero bits.
-	*/
-CPU_LE( rev	syndrome, syndrome )
-CPU_LE( rev	data1, data1 )
-CPU_LE( rev	data2, data2 )
-
-	/*
-	* For big-endian we cannot use the trick with the syndrome value
-	* as carry-propagation can corrupt the upper bits if the trailing
-	* bytes in the string contain 0x01.
-	* However, if there is no NUL byte in the dword, we can generate
-	* the result directly.  We cannot just subtract the bytes as the
-	* MSB might be significant.
-	*/
-CPU_BE( cbnz	has_nul, 1f )
-CPU_BE( cmp	data1, data2 )
-CPU_BE( cset	result, ne )
-CPU_BE( cneg	result, result, lo )
-CPU_BE( ret )
-CPU_BE( 1: )
-	/*Re-compute the NUL-byte detection, using a byte-reversed value. */
-CPU_BE(	rev	tmp3, data1 )
-CPU_BE(	sub	tmp1, tmp3, zeroones )
-CPU_BE(	orr	tmp2, tmp3, #REP8_7f )
-CPU_BE(	bic	has_nul, tmp1, tmp2 )
-CPU_BE(	rev	has_nul, has_nul )
-CPU_BE(	orr	syndrome, diff, has_nul )
-
-	clz	pos, syndrome
-	/*
-	* The MS-non-zero bit of the syndrome marks either the first bit
-	* that is different, or the top bit of the first zero byte.
-	* Shifting left now will bring the critical information into the
-	* top bits.
-	*/
-	lsl	data1, data1, pos
-	lsl	data2, data2, pos
-	/*
-	* But we need to zero-extend (char is unsigned) the value and then
-	* perform a signed 32-bit subtraction.
-	*/
-	lsr	data1, data1, #56
-	sub	result, data1, data2, lsr #56
-	ret
 SYM_FUNC_END_PI(strcmp)
 EXPORT_SYMBOL_NOKASAN(strcmp)
-- 
2.21.0.dirty


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

[PATCH v2 3/8] arm64: Import updated version of Cortex Strings' strlen

[Robin Murphy]

From: Sam Tebbs <sam.tebbs@arm.com>

Import an updated version of the former Cortex Strings - now Arm
Optimized Routines - strcmp function. The latest version introduces
Advanced SIMD usage which rules it out for our purposes, but we can
still pick an intermediate improvement from the previous version,
namely string/aarch64/strlen.S at commit 98e4d6a from
https://github.com/ARM-software/optimized-routines

Note that for simplicity Arm have chosen to contribute this code
to Linux under GPLv2 rather than the original MIT license.

Signed-off-by: Sam Tebbs <sam.tebbs@arm.com>
[ rm: update attribution and commit message ]
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
 arch/arm64/lib/strlen.S | 262 +++++++++++++++++++++++++++-------------
 1 file changed, 175 insertions(+), 87 deletions(-)

diff --git a/arch/arm64/lib/strlen.S b/arch/arm64/lib/strlen.S
index ee3ed882dd79..b557185b54a5 100644
--- a/arch/arm64/lib/strlen.S
+++ b/arch/arm64/lib/strlen.S
@@ -1,115 +1,203 @@
 /* SPDX-License-Identifier: GPL-2.0-only */
 /*
- * Copyright (C) 2013 ARM Ltd.
- * Copyright (C) 2013 Linaro.
+ * Copyright (c) 2013, Arm Limited.
  *
- * This code is based on glibc cortex strings work originally authored by Linaro
- * be found @
- *
- * http://bazaar.launchpad.net/~linaro-toolchain-dev/cortex-strings/trunk/
- * files/head:/src/aarch64/
+ * Adapted from the original at:
+ * https://github.com/ARM-software/optimized-routines/blob/master/string/aarch64/strlen.S
  */
 
 #include <linux/linkage.h>
 #include <asm/assembler.h>
 
-/*
- * calculate the length of a string
+/* Assumptions:
  *
- * Parameters:
- *	x0 - const string pointer
- * Returns:
- *	x0 - the return length of specific string
+ * ARMv8-a, AArch64, unaligned accesses, min page size 4k.
  */
 
+#define L(label) .L ## label
+
 /* Arguments and results.  */
-srcin		.req	x0
-len		.req	x0
+#define srcin		x0
+#define len		x0
 
 /* Locals and temporaries.  */
-src		.req	x1
-data1		.req	x2
-data2		.req	x3
-data2a		.req	x4
-has_nul1	.req	x5
-has_nul2	.req	x6
-tmp1		.req	x7
-tmp2		.req	x8
-tmp3		.req	x9
-tmp4		.req	x10
-zeroones	.req	x11
-pos		.req	x12
+#define src		x1
+#define data1		x2
+#define data2		x3
+#define has_nul1	x4
+#define has_nul2	x5
+#define tmp1		x4
+#define tmp2		x5
+#define tmp3		x6
+#define tmp4		x7
+#define zeroones	x8
+
+	/* NUL detection works on the principle that (X - 1) & (~X) & 0x80
+	   (=> (X - 1) & ~(X | 0x7f)) is non-zero iff a byte is zero, and
+	   can be done in parallel across the entire word. A faster check
+	   (X - 1) & 0x80 is zero for non-NUL ASCII characters, but gives
+	   false hits for characters 129..255.	*/
 
 #define REP8_01 0x0101010101010101
 #define REP8_7f 0x7f7f7f7f7f7f7f7f
 #define REP8_80 0x8080808080808080
 
+#define MIN_PAGE_SIZE 4096
+
+	/* Since strings are short on average, we check the first 16 bytes
+	   of the string for a NUL character.  In order to do an unaligned ldp
+	   safely we have to do a page cross check first.  If there is a NUL
+	   byte we calculate the length from the 2 8-byte words using
+	   conditional select to reduce branch mispredictions (it is unlikely
+	   strlen will be repeatedly called on strings with the same length).
+
+	   If the string is longer than 16 bytes, we align src so don't need
+	   further page cross checks, and process 32 bytes per iteration
+	   using the fast NUL check.  If we encounter non-ASCII characters,
+	   fallback to a second loop using the full NUL check.
+
+	   If the page cross check fails, we read 16 bytes from an aligned
+	   address, remove any characters before the string, and continue
+	   in the main loop using aligned loads.  Since strings crossing a
+	   page in the first 16 bytes are rare (probability of
+	   16/MIN_PAGE_SIZE ~= 0.4%), this case does not need to be optimized.
+
+	   AArch64 systems have a minimum page size of 4k.  We don't bother
+	   checking for larger page sizes - the cost of setting up the correct
+	   page size is just not worth the extra gain from a small reduction in
+	   the cases taking the slow path.  Note that we only care about
+	   whether the first fetch, which may be misaligned, crosses a page
+	   boundary.  */
+
 SYM_FUNC_START_WEAK_PI(strlen)
-	mov	zeroones, #REP8_01
-	bic	src, srcin, #15
-	ands	tmp1, srcin, #15
-	b.ne	.Lmisaligned
-	/*
-	* NUL detection works on the principle that (X - 1) & (~X) & 0x80
-	* (=> (X - 1) & ~(X | 0x7f)) is non-zero iff a byte is zero, and
-	* can be done in parallel across the entire word.
-	*/
-	/*
-	* The inner loop deals with two Dwords at a time. This has a
-	* slightly higher start-up cost, but we should win quite quickly,
-	* especially on cores with a high number of issue slots per
-	* cycle, as we get much better parallelism out of the operations.
-	*/
-.Lloop:
-	ldp	data1, data2, [src], #16
-.Lrealigned:
+	and	tmp1, srcin, MIN_PAGE_SIZE - 1
+	mov	zeroones, REP8_01
+	cmp	tmp1, MIN_PAGE_SIZE - 16
+	b.gt	L(page_cross)
+	ldp	data1, data2, [srcin]
+#ifdef __AARCH64EB__
+	/* For big-endian, carry propagation (if the final byte in the
+	   string is 0x01) means we cannot use has_nul1/2 directly.
+	   Since we expect strings to be small and early-exit,
+	   byte-swap the data now so has_null1/2 will be correct.  */
+	rev	data1, data1
+	rev	data2, data2
+#endif
 	sub	tmp1, data1, zeroones
-	orr	tmp2, data1, #REP8_7f
+	orr	tmp2, data1, REP8_7f
 	sub	tmp3, data2, zeroones
-	orr	tmp4, data2, #REP8_7f
-	bic	has_nul1, tmp1, tmp2
-	bics	has_nul2, tmp3, tmp4
-	ccmp	has_nul1, #0, #0, eq	/* NZCV = 0000  */
-	b.eq	.Lloop
+	orr	tmp4, data2, REP8_7f
+	bics	has_nul1, tmp1, tmp2
+	bic	has_nul2, tmp3, tmp4
+	ccmp	has_nul2, 0, 0, eq
+	beq	L(main_loop_entry)
 
-	sub	len, src, srcin
-	cbz	has_nul1, .Lnul_in_data2
-CPU_BE(	mov	data2, data1 )	/*prepare data to re-calculate the syndrome*/
-	sub	len, len, #8
-	mov	has_nul2, has_nul1
-.Lnul_in_data2:
-	/*
-	* For big-endian, carry propagation (if the final byte in the
-	* string is 0x01) means we cannot use has_nul directly.  The
-	* easiest way to get the correct byte is to byte-swap the data
-	* and calculate the syndrome a second time.
-	*/
-CPU_BE( rev	data2, data2 )
-CPU_BE( sub	tmp1, data2, zeroones )
-CPU_BE( orr	tmp2, data2, #REP8_7f )
-CPU_BE( bic	has_nul2, tmp1, tmp2 )
-
-	sub	len, len, #8
-	rev	has_nul2, has_nul2
-	clz	pos, has_nul2
-	add	len, len, pos, lsr #3		/* Bits to bytes.  */
+	/* Enter with C = has_nul1 == 0.  */
+	csel	has_nul1, has_nul1, has_nul2, cc
+	mov	len, 8
+	rev	has_nul1, has_nul1
+	clz	tmp1, has_nul1
+	csel	len, xzr, len, cc
+	add	len, len, tmp1, lsr 3
 	ret
 
-.Lmisaligned:
-	cmp	tmp1, #8
-	neg	tmp1, tmp1
-	ldp	data1, data2, [src], #16
-	lsl	tmp1, tmp1, #3		/* Bytes beyond alignment -> bits.  */
-	mov	tmp2, #~0
-	/* Big-endian.  Early bytes are at MSB.  */
-CPU_BE( lsl	tmp2, tmp2, tmp1 )	/* Shift (tmp1 & 63).  */
-	/* Little-endian.  Early bytes are at LSB.  */
-CPU_LE( lsr	tmp2, tmp2, tmp1 )	/* Shift (tmp1 & 63).  */
+	/* The inner loop processes 32 bytes per iteration and uses the fast
+	   NUL check.  If we encounter non-ASCII characters, use a second
+	   loop with the accurate NUL check.  */
+	.p2align 4
+L(main_loop_entry):
+	bic	src, srcin, 15
+	sub	src, src, 16
+L(main_loop):
+	ldp	data1, data2, [src, 32]!
+L(page_cross_entry):
+	sub	tmp1, data1, zeroones
+	sub	tmp3, data2, zeroones
+	orr	tmp2, tmp1, tmp3
+	tst	tmp2, zeroones, lsl 7
+	bne	1f
+	ldp	data1, data2, [src, 16]
+	sub	tmp1, data1, zeroones
+	sub	tmp3, data2, zeroones
+	orr	tmp2, tmp1, tmp3
+	tst	tmp2, zeroones, lsl 7
+	beq	L(main_loop)
+	add	src, src, 16
+1:
+	/* The fast check failed, so do the slower, accurate NUL check.	 */
+	orr	tmp2, data1, REP8_7f
+	orr	tmp4, data2, REP8_7f
+	bics	has_nul1, tmp1, tmp2
+	bic	has_nul2, tmp3, tmp4
+	ccmp	has_nul2, 0, 0, eq
+	beq	L(nonascii_loop)
+
+	/* Enter with C = has_nul1 == 0.  */
+L(tail):
+#ifdef __AARCH64EB__
+	/* For big-endian, carry propagation (if the final byte in the
+	   string is 0x01) means we cannot use has_nul1/2 directly.  The
+	   easiest way to get the correct byte is to byte-swap the data
+	   and calculate the syndrome a second time.  */
+	csel	data1, data1, data2, cc
+	rev	data1, data1
+	sub	tmp1, data1, zeroones
+	orr	tmp2, data1, REP8_7f
+	bic	has_nul1, tmp1, tmp2
+#else
+	csel	has_nul1, has_nul1, has_nul2, cc
+#endif
+	sub	len, src, srcin
+	rev	has_nul1, has_nul1
+	add	tmp2, len, 8
+	clz	tmp1, has_nul1
+	csel	len, len, tmp2, cc
+	add	len, len, tmp1, lsr 3
+	ret
+
+L(nonascii_loop):
+	ldp	data1, data2, [src, 16]!
+	sub	tmp1, data1, zeroones
+	orr	tmp2, data1, REP8_7f
+	sub	tmp3, data2, zeroones
+	orr	tmp4, data2, REP8_7f
+	bics	has_nul1, tmp1, tmp2
+	bic	has_nul2, tmp3, tmp4
+	ccmp	has_nul2, 0, 0, eq
+	bne	L(tail)
+	ldp	data1, data2, [src, 16]!
+	sub	tmp1, data1, zeroones
+	orr	tmp2, data1, REP8_7f
+	sub	tmp3, data2, zeroones
+	orr	tmp4, data2, REP8_7f
+	bics	has_nul1, tmp1, tmp2
+	bic	has_nul2, tmp3, tmp4
+	ccmp	has_nul2, 0, 0, eq
+	beq	L(nonascii_loop)
+	b	L(tail)
+
+	/* Load 16 bytes from [srcin & ~15] and force the bytes that precede
+	   srcin to 0x7f, so we ignore any NUL bytes before the string.
+	   Then continue in the aligned loop.  */
+L(page_cross):
+	bic	src, srcin, 15
+	ldp	data1, data2, [src]
+	lsl	tmp1, srcin, 3
+	mov	tmp4, -1
+#ifdef __AARCH64EB__
+	/* Big-endian.	Early bytes are at MSB.	 */
+	lsr	tmp1, tmp4, tmp1	/* Shift (tmp1 & 63).  */
+#else
+	/* Little-endian.  Early bytes are at LSB.  */
+	lsl	tmp1, tmp4, tmp1	/* Shift (tmp1 & 63).  */
+#endif
+	orr	tmp1, tmp1, REP8_80
+	orn	data1, data1, tmp1
+	orn	tmp2, data2, tmp1
+	tst	srcin, 8
+	csel	data1, data1, tmp4, eq
+	csel	data2, data2, tmp2, eq
+	b	L(page_cross_entry)
 
-	orr	data1, data1, tmp2
-	orr	data2a, data2, tmp2
-	csinv	data1, data1, xzr, le
-	csel	data2, data2, data2a, le
-	b	.Lrealigned
 SYM_FUNC_END_PI(strlen)
 EXPORT_SYMBOL_NOKASAN(strlen)
-- 
2.21.0.dirty


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

[PATCH v2 4/8] arm64: Import latest version of Cortex Strings' strncmp

[Robin Murphy]

From: Sam Tebbs <sam.tebbs@arm.com>

Import the latest version of the former Cortex Strings - now
Arm Optimized Routines - strncmp function based on the upstream
code of string/aarch64/strncmp.S at commit e823e3a from
https://github.com/ARM-software/optimized-routines

Note that for simplicity Arm have chosen to contribute this code
to Linux under GPLv2 rather than the original MIT license.

Signed-off-by: Sam Tebbs <sam.tebbs@arm.com>
[ rm: update attribution and commit message ]
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
 arch/arm64/lib/strncmp.S | 436 ++++++++++++++++++---------------------
 1 file changed, 199 insertions(+), 237 deletions(-)

diff --git a/arch/arm64/lib/strncmp.S b/arch/arm64/lib/strncmp.S
index 2a7ee949ed47..0c0bf5462de0 100644
--- a/arch/arm64/lib/strncmp.S
+++ b/arch/arm64/lib/strncmp.S
@@ -1,299 +1,261 @@
 /* SPDX-License-Identifier: GPL-2.0-only */
 /*
- * Copyright (C) 2013 ARM Ltd.
- * Copyright (C) 2013 Linaro.
+ * Copyright (c) 2013, Arm Limited.
  *
- * This code is based on glibc cortex strings work originally authored by Linaro
- * be found @
- *
- * http://bazaar.launchpad.net/~linaro-toolchain-dev/cortex-strings/trunk/
- * files/head:/src/aarch64/
+ * Adapted from the original at:
+ * https://github.com/ARM-software/optimized-routines/blob/master/string/aarch64/strncmp.S
  */
 
 #include <linux/linkage.h>
 #include <asm/assembler.h>
 
-/*
- * compare two strings
+/* Assumptions:
  *
- * Parameters:
- *  x0 - const string 1 pointer
- *  x1 - const string 2 pointer
- *  x2 - the maximal length to be compared
- * Returns:
- *  x0 - an integer less than, equal to, or greater than zero if s1 is found,
- *     respectively, to be less than, to match, or be greater than s2.
+ * ARMv8-a, AArch64
  */
 
+#define L(label) .L ## label
+
 #define REP8_01 0x0101010101010101
 #define REP8_7f 0x7f7f7f7f7f7f7f7f
 #define REP8_80 0x8080808080808080
 
 /* Parameters and result.  */
-src1		.req	x0
-src2		.req	x1
-limit		.req	x2
-result		.req	x0
+#define src1		x0
+#define src2		x1
+#define limit		x2
+#define result		x0
 
 /* Internal variables.  */
-data1		.req	x3
-data1w		.req	w3
-data2		.req	x4
-data2w		.req	w4
-has_nul		.req	x5
-diff		.req	x6
-syndrome	.req	x7
-tmp1		.req	x8
-tmp2		.req	x9
-tmp3		.req	x10
-zeroones	.req	x11
-pos		.req	x12
-limit_wd	.req	x13
-mask		.req	x14
-endloop		.req	x15
+#define data1		x3
+#define data1w		w3
+#define data2		x4
+#define data2w		w4
+#define has_nul		x5
+#define diff		x6
+#define syndrome	x7
+#define tmp1		x8
+#define tmp2		x9
+#define tmp3		x10
+#define zeroones	x11
+#define pos		x12
+#define limit_wd	x13
+#define mask		x14
+#define endloop		x15
+#define count		mask
 
 SYM_FUNC_START_WEAK_PI(strncmp)
-	cbz	limit, .Lret0
+	cbz	limit, L(ret0)
 	eor	tmp1, src1, src2
 	mov	zeroones, #REP8_01
 	tst	tmp1, #7
-	b.ne	.Lmisaligned8
-	ands	tmp1, src1, #7
-	b.ne	.Lmutual_align
+	and	count, src1, #7
+	b.ne	L(misaligned8)
+	cbnz	count, L(mutual_align)
 	/* Calculate the number of full and partial words -1.  */
-	/*
-	* when limit is mulitply of 8, if not sub 1,
-	* the judgement of last dword will wrong.
-	*/
-	sub	limit_wd, limit, #1 /* limit != 0, so no underflow.  */
-	lsr	limit_wd, limit_wd, #3  /* Convert to Dwords.  */
+	sub	limit_wd, limit, #1	/* limit != 0, so no underflow.  */
+	lsr	limit_wd, limit_wd, #3	/* Convert to Dwords.  */
 
-	/*
-	* NUL detection works on the principle that (X - 1) & (~X) & 0x80
-	* (=> (X - 1) & ~(X | 0x7f)) is non-zero iff a byte is zero, and
-	* can be done in parallel across the entire word.
-	*/
-.Lloop_aligned:
+	/* NUL detection works on the principle that (X - 1) & (~X) & 0x80
+	   (=> (X - 1) & ~(X | 0x7f)) is non-zero iff a byte is zero, and
+	   can be done in parallel across the entire word.  */
+	.p2align 4
+L(loop_aligned):
 	ldr	data1, [src1], #8
 	ldr	data2, [src2], #8
-.Lstart_realigned:
+L(start_realigned):
 	subs	limit_wd, limit_wd, #1
 	sub	tmp1, data1, zeroones
 	orr	tmp2, data1, #REP8_7f
-	eor	diff, data1, data2  /* Non-zero if differences found.  */
-	csinv	endloop, diff, xzr, pl  /* Last Dword or differences.*/
-	bics	has_nul, tmp1, tmp2 /* Non-zero if NUL terminator.  */
+	eor	diff, data1, data2	/* Non-zero if differences found.  */
+	csinv	endloop, diff, xzr, pl	/* Last Dword or differences.  */
+	bics	has_nul, tmp1, tmp2	/* Non-zero if NUL terminator.  */
 	ccmp	endloop, #0, #0, eq
-	b.eq	.Lloop_aligned
+	b.eq	L(loop_aligned)
+	/* End of main loop */
 
-	/*Not reached the limit, must have found the end or a diff.  */
-	tbz	limit_wd, #63, .Lnot_limit
+	/* Not reached the limit, must have found the end or a diff.  */
+	tbz	limit_wd, #63, L(not_limit)
 
 	/* Limit % 8 == 0 => all bytes significant.  */
 	ands	limit, limit, #7
-	b.eq	.Lnot_limit
+	b.eq	L(not_limit)
 
-	lsl	limit, limit, #3    /* Bits -> bytes.  */
+	lsl	limit, limit, #3	/* Bits -> bytes.  */
 	mov	mask, #~0
-CPU_BE( lsr	mask, mask, limit )
-CPU_LE( lsl	mask, mask, limit )
+#ifdef __AARCH64EB__
+	lsr	mask, mask, limit
+#else
+	lsl	mask, mask, limit
+#endif
 	bic	data1, data1, mask
 	bic	data2, data2, mask
 
 	/* Make sure that the NUL byte is marked in the syndrome.  */
 	orr	has_nul, has_nul, mask
 
-.Lnot_limit:
+L(not_limit):
 	orr	syndrome, diff, has_nul
-	b	.Lcal_cmpresult
 
-.Lmutual_align:
-	/*
-	* Sources are mutually aligned, but are not currently at an
-	* alignment boundary.  Round down the addresses and then mask off
-	* the bytes that precede the start point.
-	* We also need to adjust the limit calculations, but without
-	* overflowing if the limit is near ULONG_MAX.
-	*/
-	bic	src1, src1, #7
-	bic	src2, src2, #7
-	ldr	data1, [src1], #8
-	neg	tmp3, tmp1, lsl #3  /* 64 - bits(bytes beyond align). */
-	ldr	data2, [src2], #8
-	mov	tmp2, #~0
-	sub	limit_wd, limit, #1 /* limit != 0, so no underflow.  */
-	/* Big-endian.  Early bytes are at MSB.  */
-CPU_BE( lsl	tmp2, tmp2, tmp3 )	/* Shift (tmp1 & 63).  */
-	/* Little-endian.  Early bytes are at LSB.  */
-CPU_LE( lsr	tmp2, tmp2, tmp3 )	/* Shift (tmp1 & 63).  */
-
-	and	tmp3, limit_wd, #7
-	lsr	limit_wd, limit_wd, #3
-	/* Adjust the limit. Only low 3 bits used, so overflow irrelevant.*/
-	add	limit, limit, tmp1
-	add	tmp3, tmp3, tmp1
-	orr	data1, data1, tmp2
-	orr	data2, data2, tmp2
-	add	limit_wd, limit_wd, tmp3, lsr #3
-	b	.Lstart_realigned
-
-/*when src1 offset is not equal to src2 offset...*/
-.Lmisaligned8:
-	cmp	limit, #8
-	b.lo	.Ltiny8proc /*limit < 8... */
-	/*
-	* Get the align offset length to compare per byte first.
-	* After this process, one string's address will be aligned.*/
-	and	tmp1, src1, #7
-	neg	tmp1, tmp1
-	add	tmp1, tmp1, #8
-	and	tmp2, src2, #7
-	neg	tmp2, tmp2
-	add	tmp2, tmp2, #8
-	subs	tmp3, tmp1, tmp2
-	csel	pos, tmp1, tmp2, hi /*Choose the maximum. */
-	/*
-	* Here, limit is not less than 8, so directly run .Ltinycmp
-	* without checking the limit.*/
-	sub	limit, limit, pos
-.Ltinycmp:
-	ldrb	data1w, [src1], #1
-	ldrb	data2w, [src2], #1
-	subs	pos, pos, #1
-	ccmp	data1w, #1, #0, ne  /* NZCV = 0b0000.  */
-	ccmp	data1w, data2w, #0, cs  /* NZCV = 0b0000.  */
-	b.eq	.Ltinycmp
-	cbnz	pos, 1f /*find the null or unequal...*/
-	cmp	data1w, #1
-	ccmp	data1w, data2w, #0, cs
-	b.eq	.Lstart_align /*the last bytes are equal....*/
-1:
-	sub	result, data1, data2
-	ret
-
-.Lstart_align:
-	lsr	limit_wd, limit, #3
-	cbz	limit_wd, .Lremain8
-	/*process more leading bytes to make str1 aligned...*/
-	ands	xzr, src1, #7
-	b.eq	.Lrecal_offset
-	add	src1, src1, tmp3	/*tmp3 is positive in this branch.*/
-	add	src2, src2, tmp3
-	ldr	data1, [src1], #8
-	ldr	data2, [src2], #8
-
-	sub	limit, limit, tmp3
-	lsr	limit_wd, limit, #3
-	subs	limit_wd, limit_wd, #1
-
-	sub	tmp1, data1, zeroones
-	orr	tmp2, data1, #REP8_7f
-	eor	diff, data1, data2  /* Non-zero if differences found.  */
-	csinv	endloop, diff, xzr, ne/*if limit_wd is 0,will finish the cmp*/
-	bics	has_nul, tmp1, tmp2
-	ccmp	endloop, #0, #0, eq /*has_null is ZERO: no null byte*/
-	b.ne	.Lunequal_proc
-	/*How far is the current str2 from the alignment boundary...*/
-	and	tmp3, tmp3, #7
-.Lrecal_offset:
-	neg	pos, tmp3
-.Lloopcmp_proc:
-	/*
-	* Divide the eight bytes into two parts. First,backwards the src2
-	* to an alignment boundary,load eight bytes from the SRC2 alignment
-	* boundary,then compare with the relative bytes from SRC1.
-	* If all 8 bytes are equal,then start the second part's comparison.
-	* Otherwise finish the comparison.
-	* This special handle can garantee all the accesses are in the
-	* thread/task space in avoid to overrange access.
-	*/
-	ldr	data1, [src1,pos]
-	ldr	data2, [src2,pos]
-	sub	tmp1, data1, zeroones
-	orr	tmp2, data1, #REP8_7f
-	bics	has_nul, tmp1, tmp2 /* Non-zero if NUL terminator.  */
-	eor	diff, data1, data2  /* Non-zero if differences found.  */
-	csinv	endloop, diff, xzr, eq
-	cbnz	endloop, .Lunequal_proc
-
-	/*The second part process*/
-	ldr	data1, [src1], #8
-	ldr	data2, [src2], #8
-	subs	limit_wd, limit_wd, #1
-	sub	tmp1, data1, zeroones
-	orr	tmp2, data1, #REP8_7f
-	eor	diff, data1, data2  /* Non-zero if differences found.  */
-	csinv	endloop, diff, xzr, ne/*if limit_wd is 0,will finish the cmp*/
-	bics	has_nul, tmp1, tmp2
-	ccmp	endloop, #0, #0, eq /*has_null is ZERO: no null byte*/
-	b.eq	.Lloopcmp_proc
-
-.Lunequal_proc:
-	orr	syndrome, diff, has_nul
-	cbz	syndrome, .Lremain8
-.Lcal_cmpresult:
-	/*
-	* reversed the byte-order as big-endian,then CLZ can find the most
-	* significant zero bits.
-	*/
-CPU_LE( rev	syndrome, syndrome )
-CPU_LE( rev	data1, data1 )
-CPU_LE( rev	data2, data2 )
-	/*
-	* For big-endian we cannot use the trick with the syndrome value
-	* as carry-propagation can corrupt the upper bits if the trailing
-	* bytes in the string contain 0x01.
-	* However, if there is no NUL byte in the dword, we can generate
-	* the result directly.  We can't just subtract the bytes as the
-	* MSB might be significant.
-	*/
-CPU_BE( cbnz	has_nul, 1f )
-CPU_BE( cmp	data1, data2 )
-CPU_BE( cset	result, ne )
-CPU_BE( cneg	result, result, lo )
-CPU_BE( ret )
-CPU_BE( 1: )
-	/* Re-compute the NUL-byte detection, using a byte-reversed value.*/
-CPU_BE( rev	tmp3, data1 )
-CPU_BE( sub	tmp1, tmp3, zeroones )
-CPU_BE( orr	tmp2, tmp3, #REP8_7f )
-CPU_BE( bic	has_nul, tmp1, tmp2 )
-CPU_BE( rev	has_nul, has_nul )
-CPU_BE( orr	syndrome, diff, has_nul )
-	/*
-	* The MS-non-zero bit of the syndrome marks either the first bit
-	* that is different, or the top bit of the first zero byte.
-	* Shifting left now will bring the critical information into the
-	* top bits.
-	*/
+#ifndef	__AARCH64EB__
+	rev	syndrome, syndrome
+	rev	data1, data1
+	/* The MS-non-zero bit of the syndrome marks either the first bit
+	   that is different, or the top bit of the first zero byte.
+	   Shifting left now will bring the critical information into the
+	   top bits.  */
 	clz	pos, syndrome
+	rev	data2, data2
 	lsl	data1, data1, pos
 	lsl	data2, data2, pos
-	/*
-	* But we need to zero-extend (char is unsigned) the value and then
-	* perform a signed 32-bit subtraction.
-	*/
+	/* But we need to zero-extend (char is unsigned) the value and then
+	   perform a signed 32-bit subtraction.  */
 	lsr	data1, data1, #56
 	sub	result, data1, data2, lsr #56
 	ret
+#else
+	/* For big-endian we cannot use the trick with the syndrome value
+	   as carry-propagation can corrupt the upper bits if the trailing
+	   bytes in the string contain 0x01.  */
+	/* However, if there is no NUL byte in the dword, we can generate
+	   the result directly.  We can't just subtract the bytes as the
+	   MSB might be significant.  */
+	cbnz	has_nul, 1f
+	cmp	data1, data2
+	cset	result, ne
+	cneg	result, result, lo
+	ret
+1:
+	/* Re-compute the NUL-byte detection, using a byte-reversed value.  */
+	rev	tmp3, data1
+	sub	tmp1, tmp3, zeroones
+	orr	tmp2, tmp3, #REP8_7f
+	bic	has_nul, tmp1, tmp2
+	rev	has_nul, has_nul
+	orr	syndrome, diff, has_nul
+	clz	pos, syndrome
+	/* The MS-non-zero bit of the syndrome marks either the first bit
+	   that is different, or the top bit of the first zero byte.
+	   Shifting left now will bring the critical information into the
+	   top bits.  */
+	lsl	data1, data1, pos
+	lsl	data2, data2, pos
+	/* But we need to zero-extend (char is unsigned) the value and then
+	   perform a signed 32-bit subtraction.  */
+	lsr	data1, data1, #56
+	sub	result, data1, data2, lsr #56
+	ret
+#endif
 
-.Lremain8:
-	/* Limit % 8 == 0 => all bytes significant.  */
-	ands	limit, limit, #7
-	b.eq	.Lret0
-.Ltiny8proc:
+L(mutual_align):
+	/* Sources are mutually aligned, but are not currently at an
+	   alignment boundary.  Round down the addresses and then mask off
+	   the bytes that precede the start point.
+	   We also need to adjust the limit calculations, but without
+	   overflowing if the limit is near ULONG_MAX.  */
+	bic	src1, src1, #7
+	bic	src2, src2, #7
+	ldr	data1, [src1], #8
+	neg	tmp3, count, lsl #3	/* 64 - bits(bytes beyond align). */
+	ldr	data2, [src2], #8
+	mov	tmp2, #~0
+	sub	limit_wd, limit, #1	/* limit != 0, so no underflow.  */
+#ifdef __AARCH64EB__
+	/* Big-endian.  Early bytes are at MSB.  */
+	lsl	tmp2, tmp2, tmp3	/* Shift (count & 63).  */
+#else
+	/* Little-endian.  Early bytes are at LSB.  */
+	lsr	tmp2, tmp2, tmp3	/* Shift (count & 63).  */
+#endif
+	and	tmp3, limit_wd, #7
+	lsr	limit_wd, limit_wd, #3
+	/* Adjust the limit. Only low 3 bits used, so overflow irrelevant.  */
+	add	limit, limit, count
+	add	tmp3, tmp3, count
+	orr	data1, data1, tmp2
+	orr	data2, data2, tmp2
+	add	limit_wd, limit_wd, tmp3, lsr #3
+	b	L(start_realigned)
+
+	.p2align 4
+	/* Don't bother with dwords for up to 16 bytes.  */
+L(misaligned8):
+	cmp	limit, #16
+	b.hs	L(try_misaligned_words)
+
+L(byte_loop):
+	/* Perhaps we can do better than this.  */
 	ldrb	data1w, [src1], #1
 	ldrb	data2w, [src2], #1
 	subs	limit, limit, #1
-
-	ccmp	data1w, #1, #0, ne  /* NZCV = 0b0000.  */
-	ccmp	data1w, data2w, #0, cs  /* NZCV = 0b0000.  */
-	b.eq	.Ltiny8proc
+	ccmp	data1w, #1, #0, hi	/* NZCV = 0b0000.  */
+	ccmp	data1w, data2w, #0, cs	/* NZCV = 0b0000.  */
+	b.eq	L(byte_loop)
+L(done):
 	sub	result, data1, data2
 	ret
+	/* Align the SRC1 to a dword by doing a bytewise compare and then do
+	   the dword loop.  */
+L(try_misaligned_words):
+	lsr	limit_wd, limit, #3
+	cbz	count, L(do_misaligned)
 
-.Lret0:
+	neg	count, count
+	and	count, count, #7
+	sub	limit, limit, count
+	lsr	limit_wd, limit, #3
+
+L(page_end_loop):
+	ldrb	data1w, [src1], #1
+	ldrb	data2w, [src2], #1
+	cmp	data1w, #1
+	ccmp	data1w, data2w, #0, cs	/* NZCV = 0b0000.  */
+	b.ne	L(done)
+	subs	count, count, #1
+	b.hi	L(page_end_loop)
+
+L(do_misaligned):
+	/* Prepare ourselves for the next page crossing.  Unlike the aligned
+	   loop, we fetch 1 less dword because we risk crossing bounds on
+	   SRC2.  */
+	mov	count, #8
+	subs	limit_wd, limit_wd, #1
+	b.lo	L(done_loop)
+L(loop_misaligned):
+	and	tmp2, src2, #0xff8
+	eor	tmp2, tmp2, #0xff8
+	cbz	tmp2, L(page_end_loop)
+
+	ldr	data1, [src1], #8
+	ldr	data2, [src2], #8
+	sub	tmp1, data1, zeroones
+	orr	tmp2, data1, #REP8_7f
+	eor	diff, data1, data2	/* Non-zero if differences found.  */
+	bics	has_nul, tmp1, tmp2	/* Non-zero if NUL terminator.  */
+	ccmp	diff, #0, #0, eq
+	b.ne	L(not_limit)
+	subs	limit_wd, limit_wd, #1
+	b.pl	L(loop_misaligned)
+
+L(done_loop):
+	/* We found a difference or a NULL before the limit was reached.  */
+	and	limit, limit, #7
+	cbz	limit, L(not_limit)
+	/* Read the last word.  */
+	sub	src1, src1, 8
+	sub	src2, src2, 8
+	ldr	data1, [src1, limit]
+	ldr	data2, [src2, limit]
+	sub	tmp1, data1, zeroones
+	orr	tmp2, data1, #REP8_7f
+	eor	diff, data1, data2	/* Non-zero if differences found.  */
+	bics	has_nul, tmp1, tmp2	/* Non-zero if NUL terminator.  */
+	ccmp	diff, #0, #0, eq
+	b.ne	L(not_limit)
+
+L(ret0):
 	mov	result, #0
 	ret
+
 SYM_FUNC_END_PI(strncmp)
 EXPORT_SYMBOL_NOKASAN(strncmp)
-- 
2.21.0.dirty


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

[PATCH v2 5/8] arm64: Add assembly annotations for weak-PI-alias madness

[Robin Murphy]

Add yet another set of assembly symbol annotations, this time for the
borderline-absurd situation of a function aliasing to a weak symbol
which itself also wants a position-independent alias.

Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
 arch/arm64/include/asm/linkage.h | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/arm64/include/asm/linkage.h b/arch/arm64/include/asm/linkage.h
index ba89a9af820a..9906541a6861 100644
--- a/arch/arm64/include/asm/linkage.h
+++ b/arch/arm64/include/asm/linkage.h
@@ -56,8 +56,16 @@
 		SYM_FUNC_START_ALIAS(__pi_##x);	\
 		SYM_FUNC_START_WEAK(x)
 
+#define SYM_FUNC_START_WEAK_ALIAS_PI(x)		\
+		SYM_FUNC_START_ALIAS(__pi_##x);	\
+		SYM_START(x, SYM_L_WEAK, SYM_A_ALIGN)
+
 #define SYM_FUNC_END_PI(x)			\
 		SYM_FUNC_END(x);		\
 		SYM_FUNC_END_ALIAS(__pi_##x)
 
+#define SYM_FUNC_END_ALIAS_PI(x)		\
+		SYM_FUNC_END_ALIAS(x);		\
+		SYM_FUNC_END_ALIAS(__pi_##x)
+
 #endif
-- 
2.21.0.dirty


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

[PATCH v2 6/8] arm64: Import latest memcpy()/memmove() implementation

[Robin Murphy]

Import the latest implementation of memcpy(), based on the
upstream code of string/aarch64/memcpy.S at commit afd6244 from
https://github.com/ARM-software/optimized-routines, and subsuming
memmove() in the process.

Note that for simplicity Arm have chosen to contribute this code
to Linux under GPLv2 rather than the original MIT license.

Note also that the needs of the usercopy routines vs. regular memcpy()
have now diverged so far that we abandon the shared template idea
and the damage which that incurred to the tuning of LDP/STP loops.
We'll be back to tackle those routines separately in future.

Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
 arch/arm64/lib/Makefile  |   2 +-
 arch/arm64/lib/memcpy.S  | 272 ++++++++++++++++++++++++++++++++-------
 arch/arm64/lib/memmove.S | 189 ---------------------------
 3 files changed, 230 insertions(+), 233 deletions(-)
 delete mode 100644 arch/arm64/lib/memmove.S

diff --git a/arch/arm64/lib/Makefile b/arch/arm64/lib/Makefile
index d31e1169d9b8..01c596aa539c 100644
--- a/arch/arm64/lib/Makefile
+++ b/arch/arm64/lib/Makefile
@@ -1,7 +1,7 @@
 # SPDX-License-Identifier: GPL-2.0
 lib-y		:= clear_user.o delay.o copy_from_user.o		\
 		   copy_to_user.o copy_in_user.o copy_page.o		\
-		   clear_page.o csum.o memchr.o memcpy.o memmove.o	\
+		   clear_page.o csum.o memchr.o memcpy.o		\
 		   memset.o memcmp.o strcmp.o strncmp.o strlen.o	\
 		   strnlen.o strchr.o strrchr.o tishift.o
 
diff --git a/arch/arm64/lib/memcpy.S b/arch/arm64/lib/memcpy.S
index dc8d2a216a6e..31073a8304fb 100644
--- a/arch/arm64/lib/memcpy.S
+++ b/arch/arm64/lib/memcpy.S
@@ -1,66 +1,252 @@
 /* SPDX-License-Identifier: GPL-2.0-only */
 /*
- * Copyright (C) 2013 ARM Ltd.
- * Copyright (C) 2013 Linaro.
+ * Copyright (c) 2012-2020, Arm Limited.
  *
- * This code is based on glibc cortex strings work originally authored by Linaro
- * be found @
- *
- * http://bazaar.launchpad.net/~linaro-toolchain-dev/cortex-strings/trunk/
- * files/head:/src/aarch64/
+ * Adapted from the original at:
+ * https://github.com/ARM-software/optimized-routines/blob/master/string/aarch64/memcpy.S
  */
 
 #include <linux/linkage.h>
 #include <asm/assembler.h>
-#include <asm/cache.h>
 
-/*
- * Copy a buffer from src to dest (alignment handled by the hardware)
+/* Assumptions:
+ *
+ * ARMv8-a, AArch64, unaligned accesses.
  *
- * Parameters:
- *	x0 - dest
- *	x1 - src
- *	x2 - n
- * Returns:
- *	x0 - dest
  */
-	.macro ldrb1 reg, ptr, val
-	ldrb  \reg, [\ptr], \val
-	.endm
 
-	.macro strb1 reg, ptr, val
-	strb \reg, [\ptr], \val
-	.endm
+#define L(label) .L ## label
 
-	.macro ldrh1 reg, ptr, val
-	ldrh  \reg, [\ptr], \val
-	.endm
+#define dstin	x0
+#define src	x1
+#define count	x2
+#define dst	x3
+#define srcend	x4
+#define dstend	x5
+#define A_l	x6
+#define A_lw	w6
+#define A_h	x7
+#define B_l	x8
+#define B_lw	w8
+#define B_h	x9
+#define C_l	x10
+#define C_lw	w10
+#define C_h	x11
+#define D_l	x12
+#define D_h	x13
+#define E_l	x14
+#define E_h	x15
+#define F_l	x16
+#define F_h	x17
+#define G_l	count
+#define G_h	dst
+#define H_l	src
+#define H_h	srcend
+#define tmp1	x14
 
-	.macro strh1 reg, ptr, val
-	strh \reg, [\ptr], \val
-	.endm
+/* This implementation handles overlaps and supports both memcpy and memmove
+   from a single entry point.  It uses unaligned accesses and branchless
+   sequences to keep the code small, simple and improve performance.
 
-	.macro ldr1 reg, ptr, val
-	ldr \reg, [\ptr], \val
-	.endm
+   Copies are split into 3 main cases: small copies of up to 32 bytes, medium
+   copies of up to 128 bytes, and large copies.  The overhead of the overlap
+   check is negligible since it is only required for large copies.
 
-	.macro str1 reg, ptr, val
-	str \reg, [\ptr], \val
-	.endm
-
-	.macro ldp1 reg1, reg2, ptr, val
-	ldp \reg1, \reg2, [\ptr], \val
-	.endm
-
-	.macro stp1 reg1, reg2, ptr, val
-	stp \reg1, \reg2, [\ptr], \val
-	.endm
+   Large copies use a software pipelined loop processing 64 bytes per iteration.
+   The destination pointer is 16-byte aligned to minimize unaligned accesses.
+   The loop tail is handled by always copying 64 bytes from the end.
+*/
 
+SYM_FUNC_START_ALIAS(__memmove)
+SYM_FUNC_START_WEAK_ALIAS_PI(memmove)
 SYM_FUNC_START_ALIAS(__memcpy)
 SYM_FUNC_START_WEAK_PI(memcpy)
-#include "copy_template.S"
+	add	srcend, src, count
+	add	dstend, dstin, count
+	cmp	count, 128
+	b.hi	L(copy_long)
+	cmp	count, 32
+	b.hi	L(copy32_128)
+
+	/* Small copies: 0..32 bytes.  */
+	cmp	count, 16
+	b.lo	L(copy16)
+	ldp	A_l, A_h, [src]
+	ldp	D_l, D_h, [srcend, -16]
+	stp	A_l, A_h, [dstin]
+	stp	D_l, D_h, [dstend, -16]
 	ret
+
+	/* Copy 8-15 bytes.  */
+L(copy16):
+	tbz	count, 3, L(copy8)
+	ldr	A_l, [src]
+	ldr	A_h, [srcend, -8]
+	str	A_l, [dstin]
+	str	A_h, [dstend, -8]
+	ret
+
+	.p2align 3
+	/* Copy 4-7 bytes.  */
+L(copy8):
+	tbz	count, 2, L(copy4)
+	ldr	A_lw, [src]
+	ldr	B_lw, [srcend, -4]
+	str	A_lw, [dstin]
+	str	B_lw, [dstend, -4]
+	ret
+
+	/* Copy 0..3 bytes using a branchless sequence.  */
+L(copy4):
+	cbz	count, L(copy0)
+	lsr	tmp1, count, 1
+	ldrb	A_lw, [src]
+	ldrb	C_lw, [srcend, -1]
+	ldrb	B_lw, [src, tmp1]
+	strb	A_lw, [dstin]
+	strb	B_lw, [dstin, tmp1]
+	strb	C_lw, [dstend, -1]
+L(copy0):
+	ret
+
+	.p2align 4
+	/* Medium copies: 33..128 bytes.  */
+L(copy32_128):
+	ldp	A_l, A_h, [src]
+	ldp	B_l, B_h, [src, 16]
+	ldp	C_l, C_h, [srcend, -32]
+	ldp	D_l, D_h, [srcend, -16]
+	cmp	count, 64
+	b.hi	L(copy128)
+	stp	A_l, A_h, [dstin]
+	stp	B_l, B_h, [dstin, 16]
+	stp	C_l, C_h, [dstend, -32]
+	stp	D_l, D_h, [dstend, -16]
+	ret
+
+	.p2align 4
+	/* Copy 65..128 bytes.  */
+L(copy128):
+	ldp	E_l, E_h, [src, 32]
+	ldp	F_l, F_h, [src, 48]
+	cmp	count, 96
+	b.ls	L(copy96)
+	ldp	G_l, G_h, [srcend, -64]
+	ldp	H_l, H_h, [srcend, -48]
+	stp	G_l, G_h, [dstend, -64]
+	stp	H_l, H_h, [dstend, -48]
+L(copy96):
+	stp	A_l, A_h, [dstin]
+	stp	B_l, B_h, [dstin, 16]
+	stp	E_l, E_h, [dstin, 32]
+	stp	F_l, F_h, [dstin, 48]
+	stp	C_l, C_h, [dstend, -32]
+	stp	D_l, D_h, [dstend, -16]
+	ret
+
+	.p2align 4
+	/* Copy more than 128 bytes.  */
+L(copy_long):
+	/* Use backwards copy if there is an overlap.  */
+	sub	tmp1, dstin, src
+	cbz	tmp1, L(copy0)
+	cmp	tmp1, count
+	b.lo	L(copy_long_backwards)
+
+	/* Copy 16 bytes and then align dst to 16-byte alignment.  */
+
+	ldp	D_l, D_h, [src]
+	and	tmp1, dstin, 15
+	bic	dst, dstin, 15
+	sub	src, src, tmp1
+	add	count, count, tmp1	/* Count is now 16 too large.  */
+	ldp	A_l, A_h, [src, 16]
+	stp	D_l, D_h, [dstin]
+	ldp	B_l, B_h, [src, 32]
+	ldp	C_l, C_h, [src, 48]
+	ldp	D_l, D_h, [src, 64]!
+	subs	count, count, 128 + 16	/* Test and readjust count.  */
+	b.ls	L(copy64_from_end)
+
+L(loop64):
+	stp	A_l, A_h, [dst, 16]
+	ldp	A_l, A_h, [src, 16]
+	stp	B_l, B_h, [dst, 32]
+	ldp	B_l, B_h, [src, 32]
+	stp	C_l, C_h, [dst, 48]
+	ldp	C_l, C_h, [src, 48]
+	stp	D_l, D_h, [dst, 64]!
+	ldp	D_l, D_h, [src, 64]!
+	subs	count, count, 64
+	b.hi	L(loop64)
+
+	/* Write the last iteration and copy 64 bytes from the end.  */
+L(copy64_from_end):
+	ldp	E_l, E_h, [srcend, -64]
+	stp	A_l, A_h, [dst, 16]
+	ldp	A_l, A_h, [srcend, -48]
+	stp	B_l, B_h, [dst, 32]
+	ldp	B_l, B_h, [srcend, -32]
+	stp	C_l, C_h, [dst, 48]
+	ldp	C_l, C_h, [srcend, -16]
+	stp	D_l, D_h, [dst, 64]
+	stp	E_l, E_h, [dstend, -64]
+	stp	A_l, A_h, [dstend, -48]
+	stp	B_l, B_h, [dstend, -32]
+	stp	C_l, C_h, [dstend, -16]
+	ret
+
+	.p2align 4
+
+	/* Large backwards copy for overlapping copies.
+	   Copy 16 bytes and then align dst to 16-byte alignment.  */
+L(copy_long_backwards):
+	ldp	D_l, D_h, [srcend, -16]
+	and	tmp1, dstend, 15
+	sub	srcend, srcend, tmp1
+	sub	count, count, tmp1
+	ldp	A_l, A_h, [srcend, -16]
+	stp	D_l, D_h, [dstend, -16]
+	ldp	B_l, B_h, [srcend, -32]
+	ldp	C_l, C_h, [srcend, -48]
+	ldp	D_l, D_h, [srcend, -64]!
+	sub	dstend, dstend, tmp1
+	subs	count, count, 128
+	b.ls	L(copy64_from_start)
+
+L(loop64_backwards):
+	stp	A_l, A_h, [dstend, -16]
+	ldp	A_l, A_h, [srcend, -16]
+	stp	B_l, B_h, [dstend, -32]
+	ldp	B_l, B_h, [srcend, -32]
+	stp	C_l, C_h, [dstend, -48]
+	ldp	C_l, C_h, [srcend, -48]
+	stp	D_l, D_h, [dstend, -64]!
+	ldp	D_l, D_h, [srcend, -64]!
+	subs	count, count, 64
+	b.hi	L(loop64_backwards)
+
+	/* Write the last iteration and copy 64 bytes from the start.  */
+L(copy64_from_start):
+	ldp	G_l, G_h, [src, 48]
+	stp	A_l, A_h, [dstend, -16]
+	ldp	A_l, A_h, [src, 32]
+	stp	B_l, B_h, [dstend, -32]
+	ldp	B_l, B_h, [src, 16]
+	stp	C_l, C_h, [dstend, -48]
+	ldp	C_l, C_h, [src]
+	stp	D_l, D_h, [dstend, -64]
+	stp	G_l, G_h, [dstin, 48]
+	stp	A_l, A_h, [dstin, 32]
+	stp	B_l, B_h, [dstin, 16]
+	stp	C_l, C_h, [dstin]
+	ret
+
 SYM_FUNC_END_PI(memcpy)
 EXPORT_SYMBOL(memcpy)
 SYM_FUNC_END_ALIAS(__memcpy)
 EXPORT_SYMBOL(__memcpy)
+SYM_FUNC_END_ALIAS_PI(memmove)
+EXPORT_SYMBOL(memmove)
+SYM_FUNC_END_ALIAS(__memmove)
+EXPORT_SYMBOL(__memmove)
\ No newline at end of file
diff --git a/arch/arm64/lib/memmove.S b/arch/arm64/lib/memmove.S
deleted file mode 100644
index 1035dce4bdaf..000000000000
--- a/arch/arm64/lib/memmove.S
+++ /dev/null
@@ -1,189 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0-only */
-/*
- * Copyright (C) 2013 ARM Ltd.
- * Copyright (C) 2013 Linaro.
- *
- * This code is based on glibc cortex strings work originally authored by Linaro
- * be found @
- *
- * http://bazaar.launchpad.net/~linaro-toolchain-dev/cortex-strings/trunk/
- * files/head:/src/aarch64/
- */
-
-#include <linux/linkage.h>
-#include <asm/assembler.h>
-#include <asm/cache.h>
-
-/*
- * Move a buffer from src to test (alignment handled by the hardware).
- * If dest <= src, call memcpy, otherwise copy in reverse order.
- *
- * Parameters:
- *	x0 - dest
- *	x1 - src
- *	x2 - n
- * Returns:
- *	x0 - dest
- */
-dstin	.req	x0
-src	.req	x1
-count	.req	x2
-tmp1	.req	x3
-tmp1w	.req	w3
-tmp2	.req	x4
-tmp2w	.req	w4
-tmp3	.req	x5
-tmp3w	.req	w5
-dst	.req	x6
-
-A_l	.req	x7
-A_h	.req	x8
-B_l	.req	x9
-B_h	.req	x10
-C_l	.req	x11
-C_h	.req	x12
-D_l	.req	x13
-D_h	.req	x14
-
-SYM_FUNC_START_ALIAS(__memmove)
-SYM_FUNC_START_WEAK_PI(memmove)
-	cmp	dstin, src
-	b.lo	__memcpy
-	add	tmp1, src, count
-	cmp	dstin, tmp1
-	b.hs	__memcpy		/* No overlap.  */
-
-	add	dst, dstin, count
-	add	src, src, count
-	cmp	count, #16
-	b.lo	.Ltail15  /*probably non-alignment accesses.*/
-
-	ands	tmp2, src, #15     /* Bytes to reach alignment.  */
-	b.eq	.LSrcAligned
-	sub	count, count, tmp2
-	/*
-	* process the aligned offset length to make the src aligned firstly.
-	* those extra instructions' cost is acceptable. It also make the
-	* coming accesses are based on aligned address.
-	*/
-	tbz	tmp2, #0, 1f
-	ldrb	tmp1w, [src, #-1]!
-	strb	tmp1w, [dst, #-1]!
-1:
-	tbz	tmp2, #1, 2f
-	ldrh	tmp1w, [src, #-2]!
-	strh	tmp1w, [dst, #-2]!
-2:
-	tbz	tmp2, #2, 3f
-	ldr	tmp1w, [src, #-4]!
-	str	tmp1w, [dst, #-4]!
-3:
-	tbz	tmp2, #3, .LSrcAligned
-	ldr	tmp1, [src, #-8]!
-	str	tmp1, [dst, #-8]!
-
-.LSrcAligned:
-	cmp	count, #64
-	b.ge	.Lcpy_over64
-
-	/*
-	* Deal with small copies quickly by dropping straight into the
-	* exit block.
-	*/
-.Ltail63:
-	/*
-	* Copy up to 48 bytes of data. At this point we only need the
-	* bottom 6 bits of count to be accurate.
-	*/
-	ands	tmp1, count, #0x30
-	b.eq	.Ltail15
-	cmp	tmp1w, #0x20
-	b.eq	1f
-	b.lt	2f
-	ldp	A_l, A_h, [src, #-16]!
-	stp	A_l, A_h, [dst, #-16]!
-1:
-	ldp	A_l, A_h, [src, #-16]!
-	stp	A_l, A_h, [dst, #-16]!
-2:
-	ldp	A_l, A_h, [src, #-16]!
-	stp	A_l, A_h, [dst, #-16]!
-
-.Ltail15:
-	tbz	count, #3, 1f
-	ldr	tmp1, [src, #-8]!
-	str	tmp1, [dst, #-8]!
-1:
-	tbz	count, #2, 2f
-	ldr	tmp1w, [src, #-4]!
-	str	tmp1w, [dst, #-4]!
-2:
-	tbz	count, #1, 3f
-	ldrh	tmp1w, [src, #-2]!
-	strh	tmp1w, [dst, #-2]!
-3:
-	tbz	count, #0, .Lexitfunc
-	ldrb	tmp1w, [src, #-1]
-	strb	tmp1w, [dst, #-1]
-
-.Lexitfunc:
-	ret
-
-.Lcpy_over64:
-	subs	count, count, #128
-	b.ge	.Lcpy_body_large
-	/*
-	* Less than 128 bytes to copy, so handle 64 bytes here and then jump
-	* to the tail.
-	*/
-	ldp	A_l, A_h, [src, #-16]
-	stp	A_l, A_h, [dst, #-16]
-	ldp	B_l, B_h, [src, #-32]
-	ldp	C_l, C_h, [src, #-48]
-	stp	B_l, B_h, [dst, #-32]
-	stp	C_l, C_h, [dst, #-48]
-	ldp	D_l, D_h, [src, #-64]!
-	stp	D_l, D_h, [dst, #-64]!
-
-	tst	count, #0x3f
-	b.ne	.Ltail63
-	ret
-
-	/*
-	* Critical loop. Start at a new cache line boundary. Assuming
-	* 64 bytes per line this ensures the entire loop is in one line.
-	*/
-	.p2align	L1_CACHE_SHIFT
-.Lcpy_body_large:
-	/* pre-load 64 bytes data. */
-	ldp	A_l, A_h, [src, #-16]
-	ldp	B_l, B_h, [src, #-32]
-	ldp	C_l, C_h, [src, #-48]
-	ldp	D_l, D_h, [src, #-64]!
-1:
-	/*
-	* interlace the load of next 64 bytes data block with store of the last
-	* loaded 64 bytes data.
-	*/
-	stp	A_l, A_h, [dst, #-16]
-	ldp	A_l, A_h, [src, #-16]
-	stp	B_l, B_h, [dst, #-32]
-	ldp	B_l, B_h, [src, #-32]
-	stp	C_l, C_h, [dst, #-48]
-	ldp	C_l, C_h, [src, #-48]
-	stp	D_l, D_h, [dst, #-64]!
-	ldp	D_l, D_h, [src, #-64]!
-	subs	count, count, #64
-	b.ge	1b
-	stp	A_l, A_h, [dst, #-16]
-	stp	B_l, B_h, [dst, #-32]
-	stp	C_l, C_h, [dst, #-48]
-	stp	D_l, D_h, [dst, #-64]!
-
-	tst	count, #0x3f
-	b.ne	.Ltail63
-	ret
-SYM_FUNC_END_PI(memmove)
-EXPORT_SYMBOL(memmove)
-SYM_FUNC_END_ALIAS(__memmove)
-EXPORT_SYMBOL(__memmove)
-- 
2.21.0.dirty


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

Re: [PATCH v2 6/8] arm64: Import latest memcpy()/memmove() implementation

[Marek Szyprowski]

Hi Robin,

On 27.05.2021 17:34, Robin Murphy wrote:
> Import the latest implementation of memcpy(), based on the
> upstream code of string/aarch64/memcpy.S at commit afd6244 from
> https://github.com/ARM-software/optimized-routines, and subsuming
> memmove() in the process.
>
> Note that for simplicity Arm have chosen to contribute this code
> to Linux under GPLv2 rather than the original MIT license.
>
> Note also that the needs of the usercopy routines vs. regular memcpy()
> have now diverged so far that we abandon the shared template idea
> and the damage which that incurred to the tuning of LDP/STP loops.
> We'll be back to tackle those routines separately in future.
>
> Signed-off-by: Robin Murphy <robin.murphy@arm.com>

This patch landed recently in linux-next as commit 285133040e6c ("arm64: 
Import latest memcpy()/memmove() implementation"). Sadly it causes 
serious issues on Khadas VIM3 board. Reverting it on top of linux 
next-20210607 (together with 6b8f648959e5 and resolving the conflict in 
the Makefile) fixes the issue. Here is the kernel log:

Unable to handle kernel paging request at virtual address ffff8000136bd204
Mem abort info:
   ESR = 0x96000061
   EC = 0x25: DABT (current EL), IL = 32 bits
   SET = 0, FnV = 0
   EA = 0, S1PTW = 0
Data abort info:
   ISV = 0, ISS = 0x00000061
   CM = 0, WnR = 1
swapper pgtable: 4k pages, 48-bit VAs, pgdp=0000000009da6000
[ffff8000136bd204] pgd=10000000f4806003, p4d=10000000f4806003, 
pud=10000000f4805003, pmd=1000000000365003, pte=00680000ffe03713
Internal error: Oops: 96000061 [#1] PREEMPT SMP
Modules linked in: brcmfmac brcmutil cfg80211 dw_hdmi_i2s_audio 
meson_gxl hci_uart btqca btbcm bluetooth panfrost ecdh_generic ecc 
snd_soc_meson_axg_sound_card crct10dif_ce snd_soc_meson_card_utils 
rfkill rtc_hym8563 gpu_sched dwmac_generic rc_khadas meson_gxbb_wdt 
meson_ir pwm_meson snd_soc_meson_axg_tdmin snd_soc_meson_g12a_tohdmitx 
rtc_meson_vrtc snd_soc_meson_axg_tdmout snd_soc_meson_axg_frddr 
reset_meson_audio_arb snd_soc_meson_codec_glue axg_audio meson_rng 
sclk_div dwmac_meson8b snd_soc_meson_axg_toddr mdio_mux_meson_g12a 
clk_phase stmmac_platform rng_core snd_soc_meson_axg_fifo meson_dw_hdmi 
stmmac meson_drm meson_canvas dw_hdmi pcs_xpcs display_connector 
snd_soc_meson_axg_tdm_interface nvmem_meson_efuse adc_keys 
snd_soc_meson_axg_tdm_formatter
CPU: 4 PID: 135 Comm: kworker/4:3 Not tainted 5.13.0-rc5-next-20210607 
#10441
Hardware name: Khadas VIM3 (DT)
Workqueue: events request_firmware_work_func
pstate: 20000005 (nzCv daif -PAN -UAO -TCO BTYPE=--)
pc : __memcpy+0x2c/0x260
lr : sg_copy_buffer+0x90/0x118
...
Call trace:
  __memcpy+0x2c/0x260
  sg_copy_to_buffer+0x14/0x20
  meson_mmc_start_cmd+0xf4/0x2c8
  meson_mmc_request+0x4c/0xb8
  __mmc_start_request+0xa4/0x2a8
  mmc_start_request+0x80/0xa8
  mmc_wait_for_req+0x68/0xd8
  mmc_io_rw_extended+0x1d4/0x2e0
  sdio_io_rw_ext_helper+0xb0/0x1e8
  sdio_memcpy_toio+0x20/0x28
  brcmf_sdiod_skbuff_write.isra.18+0x2c/0x68 [brcmfmac]
  brcmf_sdiod_ramrw+0xe0/0x230 [brcmfmac]
  brcmf_sdio_firmware_callback+0xa8/0x7c8 [brcmfmac]
  brcmf_fw_request_done+0x7c/0x100 [brcmfmac]
  request_firmware_work_func+0x4c/0xd8
  process_one_work+0x2a8/0x718
  worker_thread+0x48/0x460
  kthread+0x12c/0x160
  ret_from_fork+0x10/0x18
Code: 540000c3 a9401c26 a97f348c a9001c06 (a93f34ac)
---[ end trace be83fa283dc82415 ]---

I hope that the above log helps fixing the issue. IIRC the SDHCI driver 
on VIM3 board uses internal SRAM for transferring data (instead of DMA), 
so the issue is somehow related to that.

> ---
>   arch/arm64/lib/Makefile  |   2 +-
>   arch/arm64/lib/memcpy.S  | 272 ++++++++++++++++++++++++++++++++-------
>   arch/arm64/lib/memmove.S | 189 ---------------------------
>   3 files changed, 230 insertions(+), 233 deletions(-)
>   delete mode 100644 arch/arm64/lib/memmove.S
>
> diff --git a/arch/arm64/lib/Makefile b/arch/arm64/lib/Makefile
> index d31e1169d9b8..01c596aa539c 100644
> --- a/arch/arm64/lib/Makefile
> +++ b/arch/arm64/lib/Makefile
> @@ -1,7 +1,7 @@
>   # SPDX-License-Identifier: GPL-2.0
>   lib-y		:= clear_user.o delay.o copy_from_user.o		\
>   		   copy_to_user.o copy_in_user.o copy_page.o		\
> -		   clear_page.o csum.o memchr.o memcpy.o memmove.o	\
> +		   clear_page.o csum.o memchr.o memcpy.o		\
>   		   memset.o memcmp.o strcmp.o strncmp.o strlen.o	\
>   		   strnlen.o strchr.o strrchr.o tishift.o
>   
> diff --git a/arch/arm64/lib/memcpy.S b/arch/arm64/lib/memcpy.S
> index dc8d2a216a6e..31073a8304fb 100644
> --- a/arch/arm64/lib/memcpy.S
> +++ b/arch/arm64/lib/memcpy.S
> @@ -1,66 +1,252 @@
>   /* SPDX-License-Identifier: GPL-2.0-only */
>   /*
> - * Copyright (C) 2013 ARM Ltd.
> - * Copyright (C) 2013 Linaro.
> + * Copyright (c) 2012-2020, Arm Limited.
>    *
> - * This code is based on glibc cortex strings work originally authored by Linaro
> - * be found @
> - *
> - * http://bazaar.launchpad.net/~linaro-toolchain-dev/cortex-strings/trunk/
> - * files/head:/src/aarch64/
> + * Adapted from the original at:
> + * https://github.com/ARM-software/optimized-routines/blob/master/string/aarch64/memcpy.S
>    */
>   
>   #include <linux/linkage.h>
>   #include <asm/assembler.h>
> -#include <asm/cache.h>
>   
> -/*
> - * Copy a buffer from src to dest (alignment handled by the hardware)
> +/* Assumptions:
> + *
> + * ARMv8-a, AArch64, unaligned accesses.
>    *
> - * Parameters:
> - *	x0 - dest
> - *	x1 - src
> - *	x2 - n
> - * Returns:
> - *	x0 - dest
>    */
> -	.macro ldrb1 reg, ptr, val
> -	ldrb  \reg, [\ptr], \val
> -	.endm
>   
> -	.macro strb1 reg, ptr, val
> -	strb \reg, [\ptr], \val
> -	.endm
> +#define L(label) .L ## label
>   
> -	.macro ldrh1 reg, ptr, val
> -	ldrh  \reg, [\ptr], \val
> -	.endm
> +#define dstin	x0
> +#define src	x1
> +#define count	x2
> +#define dst	x3
> +#define srcend	x4
> +#define dstend	x5
> +#define A_l	x6
> +#define A_lw	w6
> +#define A_h	x7
> +#define B_l	x8
> +#define B_lw	w8
> +#define B_h	x9
> +#define C_l	x10
> +#define C_lw	w10
> +#define C_h	x11
> +#define D_l	x12
> +#define D_h	x13
> +#define E_l	x14
> +#define E_h	x15
> +#define F_l	x16
> +#define F_h	x17
> +#define G_l	count
> +#define G_h	dst
> +#define H_l	src
> +#define H_h	srcend
> +#define tmp1	x14
>   
> -	.macro strh1 reg, ptr, val
> -	strh \reg, [\ptr], \val
> -	.endm
> +/* This implementation handles overlaps and supports both memcpy and memmove
> +   from a single entry point.  It uses unaligned accesses and branchless
> +   sequences to keep the code small, simple and improve performance.
>   
> -	.macro ldr1 reg, ptr, val
> -	ldr \reg, [\ptr], \val
> -	.endm
> +   Copies are split into 3 main cases: small copies of up to 32 bytes, medium
> +   copies of up to 128 bytes, and large copies.  The overhead of the overlap
> +   check is negligible since it is only required for large copies.
>   
> -	.macro str1 reg, ptr, val
> -	str \reg, [\ptr], \val
> -	.endm
> -
> -	.macro ldp1 reg1, reg2, ptr, val
> -	ldp \reg1, \reg2, [\ptr], \val
> -	.endm
> -
> -	.macro stp1 reg1, reg2, ptr, val
> -	stp \reg1, \reg2, [\ptr], \val
> -	.endm
> +   Large copies use a software pipelined loop processing 64 bytes per iteration.
> +   The destination pointer is 16-byte aligned to minimize unaligned accesses.
> +   The loop tail is handled by always copying 64 bytes from the end.
> +*/
>   
> +SYM_FUNC_START_ALIAS(__memmove)
> +SYM_FUNC_START_WEAK_ALIAS_PI(memmove)
>   SYM_FUNC_START_ALIAS(__memcpy)
>   SYM_FUNC_START_WEAK_PI(memcpy)
> -#include "copy_template.S"
> +	add	srcend, src, count
> +	add	dstend, dstin, count
> +	cmp	count, 128
> +	b.hi	L(copy_long)
> +	cmp	count, 32
> +	b.hi	L(copy32_128)
> +
> +	/* Small copies: 0..32 bytes.  */
> +	cmp	count, 16
> +	b.lo	L(copy16)
> +	ldp	A_l, A_h, [src]
> +	ldp	D_l, D_h, [srcend, -16]
> +	stp	A_l, A_h, [dstin]
> +	stp	D_l, D_h, [dstend, -16]
>   	ret
> +
> +	/* Copy 8-15 bytes.  */
> +L(copy16):
> +	tbz	count, 3, L(copy8)
> +	ldr	A_l, [src]
> +	ldr	A_h, [srcend, -8]
> +	str	A_l, [dstin]
> +	str	A_h, [dstend, -8]
> +	ret
> +
> +	.p2align 3
> +	/* Copy 4-7 bytes.  */
> +L(copy8):
> +	tbz	count, 2, L(copy4)
> +	ldr	A_lw, [src]
> +	ldr	B_lw, [srcend, -4]
> +	str	A_lw, [dstin]
> +	str	B_lw, [dstend, -4]
> +	ret
> +
> +	/* Copy 0..3 bytes using a branchless sequence.  */
> +L(copy4):
> +	cbz	count, L(copy0)
> +	lsr	tmp1, count, 1
> +	ldrb	A_lw, [src]
> +	ldrb	C_lw, [srcend, -1]
> +	ldrb	B_lw, [src, tmp1]
> +	strb	A_lw, [dstin]
> +	strb	B_lw, [dstin, tmp1]
> +	strb	C_lw, [dstend, -1]
> +L(copy0):
> +	ret
> +
> +	.p2align 4
> +	/* Medium copies: 33..128 bytes.  */
> +L(copy32_128):
> +	ldp	A_l, A_h, [src]
> +	ldp	B_l, B_h, [src, 16]
> +	ldp	C_l, C_h, [srcend, -32]
> +	ldp	D_l, D_h, [srcend, -16]
> +	cmp	count, 64
> +	b.hi	L(copy128)
> +	stp	A_l, A_h, [dstin]
> +	stp	B_l, B_h, [dstin, 16]
> +	stp	C_l, C_h, [dstend, -32]
> +	stp	D_l, D_h, [dstend, -16]
> +	ret
> +
> +	.p2align 4
> +	/* Copy 65..128 bytes.  */
> +L(copy128):
> +	ldp	E_l, E_h, [src, 32]
> +	ldp	F_l, F_h, [src, 48]
> +	cmp	count, 96
> +	b.ls	L(copy96)
> +	ldp	G_l, G_h, [srcend, -64]
> +	ldp	H_l, H_h, [srcend, -48]
> +	stp	G_l, G_h, [dstend, -64]
> +	stp	H_l, H_h, [dstend, -48]
> +L(copy96):
> +	stp	A_l, A_h, [dstin]
> +	stp	B_l, B_h, [dstin, 16]
> +	stp	E_l, E_h, [dstin, 32]
> +	stp	F_l, F_h, [dstin, 48]
> +	stp	C_l, C_h, [dstend, -32]
> +	stp	D_l, D_h, [dstend, -16]
> +	ret
> +
> +	.p2align 4
> +	/* Copy more than 128 bytes.  */
> +L(copy_long):
> +	/* Use backwards copy if there is an overlap.  */
> +	sub	tmp1, dstin, src
> +	cbz	tmp1, L(copy0)
> +	cmp	tmp1, count
> +	b.lo	L(copy_long_backwards)
> +
> +	/* Copy 16 bytes and then align dst to 16-byte alignment.  */
> +
> +	ldp	D_l, D_h, [src]
> +	and	tmp1, dstin, 15
> +	bic	dst, dstin, 15
> +	sub	src, src, tmp1
> +	add	count, count, tmp1	/* Count is now 16 too large.  */
> +	ldp	A_l, A_h, [src, 16]
> +	stp	D_l, D_h, [dstin]
> +	ldp	B_l, B_h, [src, 32]
> +	ldp	C_l, C_h, [src, 48]
> +	ldp	D_l, D_h, [src, 64]!
> +	subs	count, count, 128 + 16	/* Test and readjust count.  */
> +	b.ls	L(copy64_from_end)
> +
> +L(loop64):
> +	stp	A_l, A_h, [dst, 16]
> +	ldp	A_l, A_h, [src, 16]
> +	stp	B_l, B_h, [dst, 32]
> +	ldp	B_l, B_h, [src, 32]
> +	stp	C_l, C_h, [dst, 48]
> +	ldp	C_l, C_h, [src, 48]
> +	stp	D_l, D_h, [dst, 64]!
> +	ldp	D_l, D_h, [src, 64]!
> +	subs	count, count, 64
> +	b.hi	L(loop64)
> +
> +	/* Write the last iteration and copy 64 bytes from the end.  */
> +L(copy64_from_end):
> +	ldp	E_l, E_h, [srcend, -64]
> +	stp	A_l, A_h, [dst, 16]
> +	ldp	A_l, A_h, [srcend, -48]
> +	stp	B_l, B_h, [dst, 32]
> +	ldp	B_l, B_h, [srcend, -32]
> +	stp	C_l, C_h, [dst, 48]
> +	ldp	C_l, C_h, [srcend, -16]
> +	stp	D_l, D_h, [dst, 64]
> +	stp	E_l, E_h, [dstend, -64]
> +	stp	A_l, A_h, [dstend, -48]
> +	stp	B_l, B_h, [dstend, -32]
> +	stp	C_l, C_h, [dstend, -16]
> +	ret
> +
> +	.p2align 4
> +
> +	/* Large backwards copy for overlapping copies.
> +	   Copy 16 bytes and then align dst to 16-byte alignment.  */
> +L(copy_long_backwards):
> +	ldp	D_l, D_h, [srcend, -16]
> +	and	tmp1, dstend, 15
> +	sub	srcend, srcend, tmp1
> +	sub	count, count, tmp1
> +	ldp	A_l, A_h, [srcend, -16]
> +	stp	D_l, D_h, [dstend, -16]
> +	ldp	B_l, B_h, [srcend, -32]
> +	ldp	C_l, C_h, [srcend, -48]
> +	ldp	D_l, D_h, [srcend, -64]!
> +	sub	dstend, dstend, tmp1
> +	subs	count, count, 128
> +	b.ls	L(copy64_from_start)
> +
> +L(loop64_backwards):
> +	stp	A_l, A_h, [dstend, -16]
> +	ldp	A_l, A_h, [srcend, -16]
> +	stp	B_l, B_h, [dstend, -32]
> +	ldp	B_l, B_h, [srcend, -32]
> +	stp	C_l, C_h, [dstend, -48]
> +	ldp	C_l, C_h, [srcend, -48]
> +	stp	D_l, D_h, [dstend, -64]!
> +	ldp	D_l, D_h, [srcend, -64]!
> +	subs	count, count, 64
> +	b.hi	L(loop64_backwards)
> +
> +	/* Write the last iteration and copy 64 bytes from the start.  */
> +L(copy64_from_start):
> +	ldp	G_l, G_h, [src, 48]
> +	stp	A_l, A_h, [dstend, -16]
> +	ldp	A_l, A_h, [src, 32]
> +	stp	B_l, B_h, [dstend, -32]
> +	ldp	B_l, B_h, [src, 16]
> +	stp	C_l, C_h, [dstend, -48]
> +	ldp	C_l, C_h, [src]
> +	stp	D_l, D_h, [dstend, -64]
> +	stp	G_l, G_h, [dstin, 48]
> +	stp	A_l, A_h, [dstin, 32]
> +	stp	B_l, B_h, [dstin, 16]
> +	stp	C_l, C_h, [dstin]
> +	ret
> +
>   SYM_FUNC_END_PI(memcpy)
>   EXPORT_SYMBOL(memcpy)
>   SYM_FUNC_END_ALIAS(__memcpy)
>   EXPORT_SYMBOL(__memcpy)
> +SYM_FUNC_END_ALIAS_PI(memmove)
> +EXPORT_SYMBOL(memmove)
> +SYM_FUNC_END_ALIAS(__memmove)
> +EXPORT_SYMBOL(__memmove)
> \ No newline at end of file
> diff --git a/arch/arm64/lib/memmove.S b/arch/arm64/lib/memmove.S
> deleted file mode 100644
> index 1035dce4bdaf..000000000000
> --- a/arch/arm64/lib/memmove.S
> +++ /dev/null
> @@ -1,189 +0,0 @@
> -/* SPDX-License-Identifier: GPL-2.0-only */
> -/*
> - * Copyright (C) 2013 ARM Ltd.
> - * Copyright (C) 2013 Linaro.
> - *
> - * This code is based on glibc cortex strings work originally authored by Linaro
> - * be found @
> - *
> - * http://bazaar.launchpad.net/~linaro-toolchain-dev/cortex-strings/trunk/
> - * files/head:/src/aarch64/
> - */
> -
> -#include <linux/linkage.h>
> -#include <asm/assembler.h>
> -#include <asm/cache.h>
> -
> -/*
> - * Move a buffer from src to test (alignment handled by the hardware).
> - * If dest <= src, call memcpy, otherwise copy in reverse order.
> - *
> - * Parameters:
> - *	x0 - dest
> - *	x1 - src
> - *	x2 - n
> - * Returns:
> - *	x0 - dest
> - */
> -dstin	.req	x0
> -src	.req	x1
> -count	.req	x2
> -tmp1	.req	x3
> -tmp1w	.req	w3
> -tmp2	.req	x4
> -tmp2w	.req	w4
> -tmp3	.req	x5
> -tmp3w	.req	w5
> -dst	.req	x6
> -
> -A_l	.req	x7
> -A_h	.req	x8
> -B_l	.req	x9
> -B_h	.req	x10
> -C_l	.req	x11
> -C_h	.req	x12
> -D_l	.req	x13
> -D_h	.req	x14
> -
> -SYM_FUNC_START_ALIAS(__memmove)
> -SYM_FUNC_START_WEAK_PI(memmove)
> -	cmp	dstin, src
> -	b.lo	__memcpy
> -	add	tmp1, src, count
> -	cmp	dstin, tmp1
> -	b.hs	__memcpy		/* No overlap.  */
> -
> -	add	dst, dstin, count
> -	add	src, src, count
> -	cmp	count, #16
> -	b.lo	.Ltail15  /*probably non-alignment accesses.*/
> -
> -	ands	tmp2, src, #15     /* Bytes to reach alignment.  */
> -	b.eq	.LSrcAligned
> -	sub	count, count, tmp2
> -	/*
> -	* process the aligned offset length to make the src aligned firstly.
> -	* those extra instructions' cost is acceptable. It also make the
> -	* coming accesses are based on aligned address.
> -	*/
> -	tbz	tmp2, #0, 1f
> -	ldrb	tmp1w, [src, #-1]!
> -	strb	tmp1w, [dst, #-1]!
> -1:
> -	tbz	tmp2, #1, 2f
> -	ldrh	tmp1w, [src, #-2]!
> -	strh	tmp1w, [dst, #-2]!
> -2:
> -	tbz	tmp2, #2, 3f
> -	ldr	tmp1w, [src, #-4]!
> -	str	tmp1w, [dst, #-4]!
> -3:
> -	tbz	tmp2, #3, .LSrcAligned
> -	ldr	tmp1, [src, #-8]!
> -	str	tmp1, [dst, #-8]!
> -
> -.LSrcAligned:
> -	cmp	count, #64
> -	b.ge	.Lcpy_over64
> -
> -	/*
> -	* Deal with small copies quickly by dropping straight into the
> -	* exit block.
> -	*/
> -.Ltail63:
> -	/*
> -	* Copy up to 48 bytes of data. At this point we only need the
> -	* bottom 6 bits of count to be accurate.
> -	*/
> -	ands	tmp1, count, #0x30
> -	b.eq	.Ltail15
> -	cmp	tmp1w, #0x20
> -	b.eq	1f
> -	b.lt	2f
> -	ldp	A_l, A_h, [src, #-16]!
> -	stp	A_l, A_h, [dst, #-16]!
> -1:
> -	ldp	A_l, A_h, [src, #-16]!
> -	stp	A_l, A_h, [dst, #-16]!
> -2:
> -	ldp	A_l, A_h, [src, #-16]!
> -	stp	A_l, A_h, [dst, #-16]!
> -
> -.Ltail15:
> -	tbz	count, #3, 1f
> -	ldr	tmp1, [src, #-8]!
> -	str	tmp1, [dst, #-8]!
> -1:
> -	tbz	count, #2, 2f
> -	ldr	tmp1w, [src, #-4]!
> -	str	tmp1w, [dst, #-4]!
> -2:
> -	tbz	count, #1, 3f
> -	ldrh	tmp1w, [src, #-2]!
> -	strh	tmp1w, [dst, #-2]!
> -3:
> -	tbz	count, #0, .Lexitfunc
> -	ldrb	tmp1w, [src, #-1]
> -	strb	tmp1w, [dst, #-1]
> -
> -.Lexitfunc:
> -	ret
> -
> -.Lcpy_over64:
> -	subs	count, count, #128
> -	b.ge	.Lcpy_body_large
> -	/*
> -	* Less than 128 bytes to copy, so handle 64 bytes here and then jump
> -	* to the tail.
> -	*/
> -	ldp	A_l, A_h, [src, #-16]
> -	stp	A_l, A_h, [dst, #-16]
> -	ldp	B_l, B_h, [src, #-32]
> -	ldp	C_l, C_h, [src, #-48]
> -	stp	B_l, B_h, [dst, #-32]
> -	stp	C_l, C_h, [dst, #-48]
> -	ldp	D_l, D_h, [src, #-64]!
> -	stp	D_l, D_h, [dst, #-64]!
> -
> -	tst	count, #0x3f
> -	b.ne	.Ltail63
> -	ret
> -
> -	/*
> -	* Critical loop. Start at a new cache line boundary. Assuming
> -	* 64 bytes per line this ensures the entire loop is in one line.
> -	*/
> -	.p2align	L1_CACHE_SHIFT
> -.Lcpy_body_large:
> -	/* pre-load 64 bytes data. */
> -	ldp	A_l, A_h, [src, #-16]
> -	ldp	B_l, B_h, [src, #-32]
> -	ldp	C_l, C_h, [src, #-48]
> -	ldp	D_l, D_h, [src, #-64]!
> -1:
> -	/*
> -	* interlace the load of next 64 bytes data block with store of the last
> -	* loaded 64 bytes data.
> -	*/
> -	stp	A_l, A_h, [dst, #-16]
> -	ldp	A_l, A_h, [src, #-16]
> -	stp	B_l, B_h, [dst, #-32]
> -	ldp	B_l, B_h, [src, #-32]
> -	stp	C_l, C_h, [dst, #-48]
> -	ldp	C_l, C_h, [src, #-48]
> -	stp	D_l, D_h, [dst, #-64]!
> -	ldp	D_l, D_h, [src, #-64]!
> -	subs	count, count, #64
> -	b.ge	1b
> -	stp	A_l, A_h, [dst, #-16]
> -	stp	B_l, B_h, [dst, #-32]
> -	stp	C_l, C_h, [dst, #-48]
> -	stp	D_l, D_h, [dst, #-64]!
> -
> -	tst	count, #0x3f
> -	b.ne	.Ltail63
> -	ret
> -SYM_FUNC_END_PI(memmove)
> -EXPORT_SYMBOL(memmove)
> -SYM_FUNC_END_ALIAS(__memmove)
> -EXPORT_SYMBOL(__memmove)

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland


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

Re: [PATCH v2 6/8] arm64: Import latest memcpy()/memmove() implementation

[Robin Murphy]

Hi Marek,

On 2021-06-08 12:15, Marek Szyprowski wrote:
> Hi Robin,
> 
> On 27.05.2021 17:34, Robin Murphy wrote:
>> Import the latest implementation of memcpy(), based on the
>> upstream code of string/aarch64/memcpy.S at commit afd6244 from
>> https://github.com/ARM-software/optimized-routines, and subsuming
>> memmove() in the process.
>>
>> Note that for simplicity Arm have chosen to contribute this code
>> to Linux under GPLv2 rather than the original MIT license.
>>
>> Note also that the needs of the usercopy routines vs. regular memcpy()
>> have now diverged so far that we abandon the shared template idea
>> and the damage which that incurred to the tuning of LDP/STP loops.
>> We'll be back to tackle those routines separately in future.
>>
>> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
> 
> This patch landed recently in linux-next as commit 285133040e6c ("arm64:
> Import latest memcpy()/memmove() implementation"). Sadly it causes
> serious issues on Khadas VIM3 board. Reverting it on top of linux
> next-20210607 (together with 6b8f648959e5 and resolving the conflict in
> the Makefile) fixes the issue. Here is the kernel log:
> 
> Unable to handle kernel paging request at virtual address ffff8000136bd204
> Mem abort info:
>     ESR = 0x96000061
>     EC = 0x25: DABT (current EL), IL = 32 bits
>     SET = 0, FnV = 0
>     EA = 0, S1PTW = 0
> Data abort info:
>     ISV = 0, ISS = 0x00000061

That's an alignment fault, which implies we're accessing something which 
isn't normal memory.

>     CM = 0, WnR = 1
> swapper pgtable: 4k pages, 48-bit VAs, pgdp=0000000009da6000
> [ffff8000136bd204] pgd=10000000f4806003, p4d=10000000f4806003,
> pud=10000000f4805003, pmd=1000000000365003, pte=00680000ffe03713
> Internal error: Oops: 96000061 [#1] PREEMPT SMP
> Modules linked in: brcmfmac brcmutil cfg80211 dw_hdmi_i2s_audio
> meson_gxl hci_uart btqca btbcm bluetooth panfrost ecdh_generic ecc
> snd_soc_meson_axg_sound_card crct10dif_ce snd_soc_meson_card_utils
> rfkill rtc_hym8563 gpu_sched dwmac_generic rc_khadas meson_gxbb_wdt
> meson_ir pwm_meson snd_soc_meson_axg_tdmin snd_soc_meson_g12a_tohdmitx
> rtc_meson_vrtc snd_soc_meson_axg_tdmout snd_soc_meson_axg_frddr
> reset_meson_audio_arb snd_soc_meson_codec_glue axg_audio meson_rng
> sclk_div dwmac_meson8b snd_soc_meson_axg_toddr mdio_mux_meson_g12a
> clk_phase stmmac_platform rng_core snd_soc_meson_axg_fifo meson_dw_hdmi
> stmmac meson_drm meson_canvas dw_hdmi pcs_xpcs display_connector
> snd_soc_meson_axg_tdm_interface nvmem_meson_efuse adc_keys
> snd_soc_meson_axg_tdm_formatter
> CPU: 4 PID: 135 Comm: kworker/4:3 Not tainted 5.13.0-rc5-next-20210607
> #10441
> Hardware name: Khadas VIM3 (DT)
> Workqueue: events request_firmware_work_func
> pstate: 20000005 (nzCv daif -PAN -UAO -TCO BTYPE=--)
> pc : __memcpy+0x2c/0x260
> lr : sg_copy_buffer+0x90/0x118
> ...
> Call trace:
>    __memcpy+0x2c/0x260
>    sg_copy_to_buffer+0x14/0x20
>    meson_mmc_start_cmd+0xf4/0x2c8
>    meson_mmc_request+0x4c/0xb8
>    __mmc_start_request+0xa4/0x2a8
>    mmc_start_request+0x80/0xa8
>    mmc_wait_for_req+0x68/0xd8
>    mmc_io_rw_extended+0x1d4/0x2e0
>    sdio_io_rw_ext_helper+0xb0/0x1e8
>    sdio_memcpy_toio+0x20/0x28
>    brcmf_sdiod_skbuff_write.isra.18+0x2c/0x68 [brcmfmac]
>    brcmf_sdiod_ramrw+0xe0/0x230 [brcmfmac]
>    brcmf_sdio_firmware_callback+0xa8/0x7c8 [brcmfmac]
>    brcmf_fw_request_done+0x7c/0x100 [brcmfmac]
>    request_firmware_work_func+0x4c/0xd8
>    process_one_work+0x2a8/0x718
>    worker_thread+0x48/0x460
>    kthread+0x12c/0x160
>    ret_from_fork+0x10/0x18
> Code: 540000c3 a9401c26 a97f348c a9001c06 (a93f34ac)
> ---[ end trace be83fa283dc82415 ]---
> 
> I hope that the above log helps fixing the issue. IIRC the SDHCI driver
> on VIM3 board uses internal SRAM for transferring data (instead of DMA),
> so the issue is somehow related to that.

Drivers shouldn't be using memcpy() on iomem mappings. Even if they 
happen to have got away with it sometimes ;)

Taking a quick look at that driver,

	host->bounce_buf = host->regs + SD_EMMC_SRAM_DATA_BUF_OFF;

is completely bogus, as Sparse will readily point out.

Robin.

>> ---
>>    arch/arm64/lib/Makefile  |   2 +-
>>    arch/arm64/lib/memcpy.S  | 272 ++++++++++++++++++++++++++++++++-------
>>    arch/arm64/lib/memmove.S | 189 ---------------------------
>>    3 files changed, 230 insertions(+), 233 deletions(-)
>>    delete mode 100644 arch/arm64/lib/memmove.S
>>
>> diff --git a/arch/arm64/lib/Makefile b/arch/arm64/lib/Makefile
>> index d31e1169d9b8..01c596aa539c 100644
>> --- a/arch/arm64/lib/Makefile
>> +++ b/arch/arm64/lib/Makefile
>> @@ -1,7 +1,7 @@
>>    # SPDX-License-Identifier: GPL-2.0
>>    lib-y		:= clear_user.o delay.o copy_from_user.o		\
>>    		   copy_to_user.o copy_in_user.o copy_page.o		\
>> -		   clear_page.o csum.o memchr.o memcpy.o memmove.o	\
>> +		   clear_page.o csum.o memchr.o memcpy.o		\
>>    		   memset.o memcmp.o strcmp.o strncmp.o strlen.o	\
>>    		   strnlen.o strchr.o strrchr.o tishift.o
>>    
>> diff --git a/arch/arm64/lib/memcpy.S b/arch/arm64/lib/memcpy.S
>> index dc8d2a216a6e..31073a8304fb 100644
>> --- a/arch/arm64/lib/memcpy.S
>> +++ b/arch/arm64/lib/memcpy.S
>> @@ -1,66 +1,252 @@
>>    /* SPDX-License-Identifier: GPL-2.0-only */
>>    /*
>> - * Copyright (C) 2013 ARM Ltd.
>> - * Copyright (C) 2013 Linaro.
>> + * Copyright (c) 2012-2020, Arm Limited.
>>     *
>> - * This code is based on glibc cortex strings work originally authored by Linaro
>> - * be found @
>> - *
>> - * http://bazaar.launchpad.net/~linaro-toolchain-dev/cortex-strings/trunk/
>> - * files/head:/src/aarch64/
>> + * Adapted from the original at:
>> + * https://github.com/ARM-software/optimized-routines/blob/master/string/aarch64/memcpy.S
>>     */
>>    
>>    #include <linux/linkage.h>
>>    #include <asm/assembler.h>
>> -#include <asm/cache.h>
>>    
>> -/*
>> - * Copy a buffer from src to dest (alignment handled by the hardware)
>> +/* Assumptions:
>> + *
>> + * ARMv8-a, AArch64, unaligned accesses.
>>     *
>> - * Parameters:
>> - *	x0 - dest
>> - *	x1 - src
>> - *	x2 - n
>> - * Returns:
>> - *	x0 - dest
>>     */
>> -	.macro ldrb1 reg, ptr, val
>> -	ldrb  \reg, [\ptr], \val
>> -	.endm
>>    
>> -	.macro strb1 reg, ptr, val
>> -	strb \reg, [\ptr], \val
>> -	.endm
>> +#define L(label) .L ## label
>>    
>> -	.macro ldrh1 reg, ptr, val
>> -	ldrh  \reg, [\ptr], \val
>> -	.endm
>> +#define dstin	x0
>> +#define src	x1
>> +#define count	x2
>> +#define dst	x3
>> +#define srcend	x4
>> +#define dstend	x5
>> +#define A_l	x6
>> +#define A_lw	w6
>> +#define A_h	x7
>> +#define B_l	x8
>> +#define B_lw	w8
>> +#define B_h	x9
>> +#define C_l	x10
>> +#define C_lw	w10
>> +#define C_h	x11
>> +#define D_l	x12
>> +#define D_h	x13
>> +#define E_l	x14
>> +#define E_h	x15
>> +#define F_l	x16
>> +#define F_h	x17
>> +#define G_l	count
>> +#define G_h	dst
>> +#define H_l	src
>> +#define H_h	srcend
>> +#define tmp1	x14
>>    
>> -	.macro strh1 reg, ptr, val
>> -	strh \reg, [\ptr], \val
>> -	.endm
>> +/* This implementation handles overlaps and supports both memcpy and memmove
>> +   from a single entry point.  It uses unaligned accesses and branchless
>> +   sequences to keep the code small, simple and improve performance.
>>    
>> -	.macro ldr1 reg, ptr, val
>> -	ldr \reg, [\ptr], \val
>> -	.endm
>> +   Copies are split into 3 main cases: small copies of up to 32 bytes, medium
>> +   copies of up to 128 bytes, and large copies.  The overhead of the overlap
>> +   check is negligible since it is only required for large copies.
>>    
>> -	.macro str1 reg, ptr, val
>> -	str \reg, [\ptr], \val
>> -	.endm
>> -
>> -	.macro ldp1 reg1, reg2, ptr, val
>> -	ldp \reg1, \reg2, [\ptr], \val
>> -	.endm
>> -
>> -	.macro stp1 reg1, reg2, ptr, val
>> -	stp \reg1, \reg2, [\ptr], \val
>> -	.endm
>> +   Large copies use a software pipelined loop processing 64 bytes per iteration.
>> +   The destination pointer is 16-byte aligned to minimize unaligned accesses.
>> +   The loop tail is handled by always copying 64 bytes from the end.
>> +*/
>>    
>> +SYM_FUNC_START_ALIAS(__memmove)
>> +SYM_FUNC_START_WEAK_ALIAS_PI(memmove)
>>    SYM_FUNC_START_ALIAS(__memcpy)
>>    SYM_FUNC_START_WEAK_PI(memcpy)
>> -#include "copy_template.S"
>> +	add	srcend, src, count
>> +	add	dstend, dstin, count
>> +	cmp	count, 128
>> +	b.hi	L(copy_long)
>> +	cmp	count, 32
>> +	b.hi	L(copy32_128)
>> +
>> +	/* Small copies: 0..32 bytes.  */
>> +	cmp	count, 16
>> +	b.lo	L(copy16)
>> +	ldp	A_l, A_h, [src]
>> +	ldp	D_l, D_h, [srcend, -16]
>> +	stp	A_l, A_h, [dstin]
>> +	stp	D_l, D_h, [dstend, -16]
>>    	ret
>> +
>> +	/* Copy 8-15 bytes.  */
>> +L(copy16):
>> +	tbz	count, 3, L(copy8)
>> +	ldr	A_l, [src]
>> +	ldr	A_h, [srcend, -8]
>> +	str	A_l, [dstin]
>> +	str	A_h, [dstend, -8]
>> +	ret
>> +
>> +	.p2align 3
>> +	/* Copy 4-7 bytes.  */
>> +L(copy8):
>> +	tbz	count, 2, L(copy4)
>> +	ldr	A_lw, [src]
>> +	ldr	B_lw, [srcend, -4]
>> +	str	A_lw, [dstin]
>> +	str	B_lw, [dstend, -4]
>> +	ret
>> +
>> +	/* Copy 0..3 bytes using a branchless sequence.  */
>> +L(copy4):
>> +	cbz	count, L(copy0)
>> +	lsr	tmp1, count, 1
>> +	ldrb	A_lw, [src]
>> +	ldrb	C_lw, [srcend, -1]
>> +	ldrb	B_lw, [src, tmp1]
>> +	strb	A_lw, [dstin]
>> +	strb	B_lw, [dstin, tmp1]
>> +	strb	C_lw, [dstend, -1]
>> +L(copy0):
>> +	ret
>> +
>> +	.p2align 4
>> +	/* Medium copies: 33..128 bytes.  */
>> +L(copy32_128):
>> +	ldp	A_l, A_h, [src]
>> +	ldp	B_l, B_h, [src, 16]
>> +	ldp	C_l, C_h, [srcend, -32]
>> +	ldp	D_l, D_h, [srcend, -16]
>> +	cmp	count, 64
>> +	b.hi	L(copy128)
>> +	stp	A_l, A_h, [dstin]
>> +	stp	B_l, B_h, [dstin, 16]
>> +	stp	C_l, C_h, [dstend, -32]
>> +	stp	D_l, D_h, [dstend, -16]
>> +	ret
>> +
>> +	.p2align 4
>> +	/* Copy 65..128 bytes.  */
>> +L(copy128):
>> +	ldp	E_l, E_h, [src, 32]
>> +	ldp	F_l, F_h, [src, 48]
>> +	cmp	count, 96
>> +	b.ls	L(copy96)
>> +	ldp	G_l, G_h, [srcend, -64]
>> +	ldp	H_l, H_h, [srcend, -48]
>> +	stp	G_l, G_h, [dstend, -64]
>> +	stp	H_l, H_h, [dstend, -48]
>> +L(copy96):
>> +	stp	A_l, A_h, [dstin]
>> +	stp	B_l, B_h, [dstin, 16]
>> +	stp	E_l, E_h, [dstin, 32]
>> +	stp	F_l, F_h, [dstin, 48]
>> +	stp	C_l, C_h, [dstend, -32]
>> +	stp	D_l, D_h, [dstend, -16]
>> +	ret
>> +
>> +	.p2align 4
>> +	/* Copy more than 128 bytes.  */
>> +L(copy_long):
>> +	/* Use backwards copy if there is an overlap.  */
>> +	sub	tmp1, dstin, src
>> +	cbz	tmp1, L(copy0)
>> +	cmp	tmp1, count
>> +	b.lo	L(copy_long_backwards)
>> +
>> +	/* Copy 16 bytes and then align dst to 16-byte alignment.  */
>> +
>> +	ldp	D_l, D_h, [src]
>> +	and	tmp1, dstin, 15
>> +	bic	dst, dstin, 15
>> +	sub	src, src, tmp1
>> +	add	count, count, tmp1	/* Count is now 16 too large.  */
>> +	ldp	A_l, A_h, [src, 16]
>> +	stp	D_l, D_h, [dstin]
>> +	ldp	B_l, B_h, [src, 32]
>> +	ldp	C_l, C_h, [src, 48]
>> +	ldp	D_l, D_h, [src, 64]!
>> +	subs	count, count, 128 + 16	/* Test and readjust count.  */
>> +	b.ls	L(copy64_from_end)
>> +
>> +L(loop64):
>> +	stp	A_l, A_h, [dst, 16]
>> +	ldp	A_l, A_h, [src, 16]
>> +	stp	B_l, B_h, [dst, 32]
>> +	ldp	B_l, B_h, [src, 32]
>> +	stp	C_l, C_h, [dst, 48]
>> +	ldp	C_l, C_h, [src, 48]
>> +	stp	D_l, D_h, [dst, 64]!
>> +	ldp	D_l, D_h, [src, 64]!
>> +	subs	count, count, 64
>> +	b.hi	L(loop64)
>> +
>> +	/* Write the last iteration and copy 64 bytes from the end.  */
>> +L(copy64_from_end):
>> +	ldp	E_l, E_h, [srcend, -64]
>> +	stp	A_l, A_h, [dst, 16]
>> +	ldp	A_l, A_h, [srcend, -48]
>> +	stp	B_l, B_h, [dst, 32]
>> +	ldp	B_l, B_h, [srcend, -32]
>> +	stp	C_l, C_h, [dst, 48]
>> +	ldp	C_l, C_h, [srcend, -16]
>> +	stp	D_l, D_h, [dst, 64]
>> +	stp	E_l, E_h, [dstend, -64]
>> +	stp	A_l, A_h, [dstend, -48]
>> +	stp	B_l, B_h, [dstend, -32]
>> +	stp	C_l, C_h, [dstend, -16]
>> +	ret
>> +
>> +	.p2align 4
>> +
>> +	/* Large backwards copy for overlapping copies.
>> +	   Copy 16 bytes and then align dst to 16-byte alignment.  */
>> +L(copy_long_backwards):
>> +	ldp	D_l, D_h, [srcend, -16]
>> +	and	tmp1, dstend, 15
>> +	sub	srcend, srcend, tmp1
>> +	sub	count, count, tmp1
>> +	ldp	A_l, A_h, [srcend, -16]
>> +	stp	D_l, D_h, [dstend, -16]
>> +	ldp	B_l, B_h, [srcend, -32]
>> +	ldp	C_l, C_h, [srcend, -48]
>> +	ldp	D_l, D_h, [srcend, -64]!
>> +	sub	dstend, dstend, tmp1
>> +	subs	count, count, 128
>> +	b.ls	L(copy64_from_start)
>> +
>> +L(loop64_backwards):
>> +	stp	A_l, A_h, [dstend, -16]
>> +	ldp	A_l, A_h, [srcend, -16]
>> +	stp	B_l, B_h, [dstend, -32]
>> +	ldp	B_l, B_h, [srcend, -32]
>> +	stp	C_l, C_h, [dstend, -48]
>> +	ldp	C_l, C_h, [srcend, -48]
>> +	stp	D_l, D_h, [dstend, -64]!
>> +	ldp	D_l, D_h, [srcend, -64]!
>> +	subs	count, count, 64
>> +	b.hi	L(loop64_backwards)
>> +
>> +	/* Write the last iteration and copy 64 bytes from the start.  */
>> +L(copy64_from_start):
>> +	ldp	G_l, G_h, [src, 48]
>> +	stp	A_l, A_h, [dstend, -16]
>> +	ldp	A_l, A_h, [src, 32]
>> +	stp	B_l, B_h, [dstend, -32]
>> +	ldp	B_l, B_h, [src, 16]
>> +	stp	C_l, C_h, [dstend, -48]
>> +	ldp	C_l, C_h, [src]
>> +	stp	D_l, D_h, [dstend, -64]
>> +	stp	G_l, G_h, [dstin, 48]
>> +	stp	A_l, A_h, [dstin, 32]
>> +	stp	B_l, B_h, [dstin, 16]
>> +	stp	C_l, C_h, [dstin]
>> +	ret
>> +
>>    SYM_FUNC_END_PI(memcpy)
>>    EXPORT_SYMBOL(memcpy)
>>    SYM_FUNC_END_ALIAS(__memcpy)
>>    EXPORT_SYMBOL(__memcpy)
>> +SYM_FUNC_END_ALIAS_PI(memmove)
>> +EXPORT_SYMBOL(memmove)
>> +SYM_FUNC_END_ALIAS(__memmove)
>> +EXPORT_SYMBOL(__memmove)
>> \ No newline at end of file
>> diff --git a/arch/arm64/lib/memmove.S b/arch/arm64/lib/memmove.S
>> deleted file mode 100644
>> index 1035dce4bdaf..000000000000
>> --- a/arch/arm64/lib/memmove.S
>> +++ /dev/null
>> @@ -1,189 +0,0 @@
>> -/* SPDX-License-Identifier: GPL-2.0-only */
>> -/*
>> - * Copyright (C) 2013 ARM Ltd.
>> - * Copyright (C) 2013 Linaro.
>> - *
>> - * This code is based on glibc cortex strings work originally authored by Linaro
>> - * be found @
>> - *
>> - * http://bazaar.launchpad.net/~linaro-toolchain-dev/cortex-strings/trunk/
>> - * files/head:/src/aarch64/
>> - */
>> -
>> -#include <linux/linkage.h>
>> -#include <asm/assembler.h>
>> -#include <asm/cache.h>
>> -
>> -/*
>> - * Move a buffer from src to test (alignment handled by the hardware).
>> - * If dest <= src, call memcpy, otherwise copy in reverse order.
>> - *
>> - * Parameters:
>> - *	x0 - dest
>> - *	x1 - src
>> - *	x2 - n
>> - * Returns:
>> - *	x0 - dest
>> - */
>> -dstin	.req	x0
>> -src	.req	x1
>> -count	.req	x2
>> -tmp1	.req	x3
>> -tmp1w	.req	w3
>> -tmp2	.req	x4
>> -tmp2w	.req	w4
>> -tmp3	.req	x5
>> -tmp3w	.req	w5
>> -dst	.req	x6
>> -
>> -A_l	.req	x7
>> -A_h	.req	x8
>> -B_l	.req	x9
>> -B_h	.req	x10
>> -C_l	.req	x11
>> -C_h	.req	x12
>> -D_l	.req	x13
>> -D_h	.req	x14
>> -
>> -SYM_FUNC_START_ALIAS(__memmove)
>> -SYM_FUNC_START_WEAK_PI(memmove)
>> -	cmp	dstin, src
>> -	b.lo	__memcpy
>> -	add	tmp1, src, count
>> -	cmp	dstin, tmp1
>> -	b.hs	__memcpy		/* No overlap.  */
>> -
>> -	add	dst, dstin, count
>> -	add	src, src, count
>> -	cmp	count, #16
>> -	b.lo	.Ltail15  /*probably non-alignment accesses.*/
>> -
>> -	ands	tmp2, src, #15     /* Bytes to reach alignment.  */
>> -	b.eq	.LSrcAligned
>> -	sub	count, count, tmp2
>> -	/*
>> -	* process the aligned offset length to make the src aligned firstly.
>> -	* those extra instructions' cost is acceptable. It also make the
>> -	* coming accesses are based on aligned address.
>> -	*/
>> -	tbz	tmp2, #0, 1f
>> -	ldrb	tmp1w, [src, #-1]!
>> -	strb	tmp1w, [dst, #-1]!
>> -1:
>> -	tbz	tmp2, #1, 2f
>> -	ldrh	tmp1w, [src, #-2]!
>> -	strh	tmp1w, [dst, #-2]!
>> -2:
>> -	tbz	tmp2, #2, 3f
>> -	ldr	tmp1w, [src, #-4]!
>> -	str	tmp1w, [dst, #-4]!
>> -3:
>> -	tbz	tmp2, #3, .LSrcAligned
>> -	ldr	tmp1, [src, #-8]!
>> -	str	tmp1, [dst, #-8]!
>> -
>> -.LSrcAligned:
>> -	cmp	count, #64
>> -	b.ge	.Lcpy_over64
>> -
>> -	/*
>> -	* Deal with small copies quickly by dropping straight into the
>> -	* exit block.
>> -	*/
>> -.Ltail63:
>> -	/*
>> -	* Copy up to 48 bytes of data. At this point we only need the
>> -	* bottom 6 bits of count to be accurate.
>> -	*/
>> -	ands	tmp1, count, #0x30
>> -	b.eq	.Ltail15
>> -	cmp	tmp1w, #0x20
>> -	b.eq	1f
>> -	b.lt	2f
>> -	ldp	A_l, A_h, [src, #-16]!
>> -	stp	A_l, A_h, [dst, #-16]!
>> -1:
>> -	ldp	A_l, A_h, [src, #-16]!
>> -	stp	A_l, A_h, [dst, #-16]!
>> -2:
>> -	ldp	A_l, A_h, [src, #-16]!
>> -	stp	A_l, A_h, [dst, #-16]!
>> -
>> -.Ltail15:
>> -	tbz	count, #3, 1f
>> -	ldr	tmp1, [src, #-8]!
>> -	str	tmp1, [dst, #-8]!
>> -1:
>> -	tbz	count, #2, 2f
>> -	ldr	tmp1w, [src, #-4]!
>> -	str	tmp1w, [dst, #-4]!
>> -2:
>> -	tbz	count, #1, 3f
>> -	ldrh	tmp1w, [src, #-2]!
>> -	strh	tmp1w, [dst, #-2]!
>> -3:
>> -	tbz	count, #0, .Lexitfunc
>> -	ldrb	tmp1w, [src, #-1]
>> -	strb	tmp1w, [dst, #-1]
>> -
>> -.Lexitfunc:
>> -	ret
>> -
>> -.Lcpy_over64:
>> -	subs	count, count, #128
>> -	b.ge	.Lcpy_body_large
>> -	/*
>> -	* Less than 128 bytes to copy, so handle 64 bytes here and then jump
>> -	* to the tail.
>> -	*/
>> -	ldp	A_l, A_h, [src, #-16]
>> -	stp	A_l, A_h, [dst, #-16]
>> -	ldp	B_l, B_h, [src, #-32]
>> -	ldp	C_l, C_h, [src, #-48]
>> -	stp	B_l, B_h, [dst, #-32]
>> -	stp	C_l, C_h, [dst, #-48]
>> -	ldp	D_l, D_h, [src, #-64]!
>> -	stp	D_l, D_h, [dst, #-64]!
>> -
>> -	tst	count, #0x3f
>> -	b.ne	.Ltail63
>> -	ret
>> -
>> -	/*
>> -	* Critical loop. Start at a new cache line boundary. Assuming
>> -	* 64 bytes per line this ensures the entire loop is in one line.
>> -	*/
>> -	.p2align	L1_CACHE_SHIFT
>> -.Lcpy_body_large:
>> -	/* pre-load 64 bytes data. */
>> -	ldp	A_l, A_h, [src, #-16]
>> -	ldp	B_l, B_h, [src, #-32]
>> -	ldp	C_l, C_h, [src, #-48]
>> -	ldp	D_l, D_h, [src, #-64]!
>> -1:
>> -	/*
>> -	* interlace the load of next 64 bytes data block with store of the last
>> -	* loaded 64 bytes data.
>> -	*/
>> -	stp	A_l, A_h, [dst, #-16]
>> -	ldp	A_l, A_h, [src, #-16]
>> -	stp	B_l, B_h, [dst, #-32]
>> -	ldp	B_l, B_h, [src, #-32]
>> -	stp	C_l, C_h, [dst, #-48]
>> -	ldp	C_l, C_h, [src, #-48]
>> -	stp	D_l, D_h, [dst, #-64]!
>> -	ldp	D_l, D_h, [src, #-64]!
>> -	subs	count, count, #64
>> -	b.ge	1b
>> -	stp	A_l, A_h, [dst, #-16]
>> -	stp	B_l, B_h, [dst, #-32]
>> -	stp	C_l, C_h, [dst, #-48]
>> -	stp	D_l, D_h, [dst, #-64]!
>> -
>> -	tst	count, #0x3f
>> -	b.ne	.Ltail63
>> -	ret
>> -SYM_FUNC_END_PI(memmove)
>> -EXPORT_SYMBOL(memmove)
>> -SYM_FUNC_END_ALIAS(__memmove)
>> -EXPORT_SYMBOL(__memmove)
> 
> Best regards
> 

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

Re: [PATCH v2 6/8] arm64: Import latest memcpy()/memmove() implementation

[Marek Szyprowski]

+ Kevin

On 08.06.2021 13:37, Robin Murphy wrote:
> Hi Marek,
>
> On 2021-06-08 12:15, Marek Szyprowski wrote:
>> Hi Robin,
>>
>> On 27.05.2021 17:34, Robin Murphy wrote:
>>> Import the latest implementation of memcpy(), based on the
>>> upstream code of string/aarch64/memcpy.S at commit afd6244 from
>>> https://protect2.fireeye.com/v1/url?k=0e25d630-51beef28-0e245d7f-0cc47a314e9a-b41fdb2d4d06ff75&q=1&e=fcfaf71d-f01a-4bc4-8e16-8ae86e0c0116&u=https%3A%2F%2Fgithub.com%2FARM-software%2Foptimized-routines, 
>>> and subsuming
>>> memmove() in the process.
>>>
>>> Note that for simplicity Arm have chosen to contribute this code
>>> to Linux under GPLv2 rather than the original MIT license.
>>>
>>> Note also that the needs of the usercopy routines vs. regular memcpy()
>>> have now diverged so far that we abandon the shared template idea
>>> and the damage which that incurred to the tuning of LDP/STP loops.
>>> We'll be back to tackle those routines separately in future.
>>>
>>> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
>>
>> This patch landed recently in linux-next as commit 285133040e6c ("arm64:
>> Import latest memcpy()/memmove() implementation"). Sadly it causes
>> serious issues on Khadas VIM3 board. Reverting it on top of linux
>> next-20210607 (together with 6b8f648959e5 and resolving the conflict in
>> the Makefile) fixes the issue. Here is the kernel log:
>>
>> Unable to handle kernel paging request at virtual address 
>> ffff8000136bd204
>> Mem abort info:
>>     ESR = 0x96000061
>>     EC = 0x25: DABT (current EL), IL = 32 bits
>>     SET = 0, FnV = 0
>>     EA = 0, S1PTW = 0
>> Data abort info:
>>     ISV = 0, ISS = 0x00000061
>
> That's an alignment fault, which implies we're accessing something 
> which isn't normal memory.
>
>>     CM = 0, WnR = 1
>> swapper pgtable: 4k pages, 48-bit VAs, pgdp=0000000009da6000
>> [ffff8000136bd204] pgd=10000000f4806003, p4d=10000000f4806003,
>> pud=10000000f4805003, pmd=1000000000365003, pte=00680000ffe03713
>> Internal error: Oops: 96000061 [#1] PREEMPT SMP
>> Modules linked in: brcmfmac brcmutil cfg80211 dw_hdmi_i2s_audio
>> meson_gxl hci_uart btqca btbcm bluetooth panfrost ecdh_generic ecc
>> snd_soc_meson_axg_sound_card crct10dif_ce snd_soc_meson_card_utils
>> rfkill rtc_hym8563 gpu_sched dwmac_generic rc_khadas meson_gxbb_wdt
>> meson_ir pwm_meson snd_soc_meson_axg_tdmin snd_soc_meson_g12a_tohdmitx
>> rtc_meson_vrtc snd_soc_meson_axg_tdmout snd_soc_meson_axg_frddr
>> reset_meson_audio_arb snd_soc_meson_codec_glue axg_audio meson_rng
>> sclk_div dwmac_meson8b snd_soc_meson_axg_toddr mdio_mux_meson_g12a
>> clk_phase stmmac_platform rng_core snd_soc_meson_axg_fifo meson_dw_hdmi
>> stmmac meson_drm meson_canvas dw_hdmi pcs_xpcs display_connector
>> snd_soc_meson_axg_tdm_interface nvmem_meson_efuse adc_keys
>> snd_soc_meson_axg_tdm_formatter
>> CPU: 4 PID: 135 Comm: kworker/4:3 Not tainted 5.13.0-rc5-next-20210607
>> #10441
>> Hardware name: Khadas VIM3 (DT)
>> Workqueue: events request_firmware_work_func
>> pstate: 20000005 (nzCv daif -PAN -UAO -TCO BTYPE=--)
>> pc : __memcpy+0x2c/0x260
>> lr : sg_copy_buffer+0x90/0x118
>> ...
>> Call trace:
>>    __memcpy+0x2c/0x260
>>    sg_copy_to_buffer+0x14/0x20
>>    meson_mmc_start_cmd+0xf4/0x2c8
>>    meson_mmc_request+0x4c/0xb8
>>    __mmc_start_request+0xa4/0x2a8
>>    mmc_start_request+0x80/0xa8
>>    mmc_wait_for_req+0x68/0xd8
>>    mmc_io_rw_extended+0x1d4/0x2e0
>>    sdio_io_rw_ext_helper+0xb0/0x1e8
>>    sdio_memcpy_toio+0x20/0x28
>>    brcmf_sdiod_skbuff_write.isra.18+0x2c/0x68 [brcmfmac]
>>    brcmf_sdiod_ramrw+0xe0/0x230 [brcmfmac]
>>    brcmf_sdio_firmware_callback+0xa8/0x7c8 [brcmfmac]
>>    brcmf_fw_request_done+0x7c/0x100 [brcmfmac]
>>    request_firmware_work_func+0x4c/0xd8
>>    process_one_work+0x2a8/0x718
>>    worker_thread+0x48/0x460
>>    kthread+0x12c/0x160
>>    ret_from_fork+0x10/0x18
>> Code: 540000c3 a9401c26 a97f348c a9001c06 (a93f34ac)
>> ---[ end trace be83fa283dc82415 ]---
>>
>> I hope that the above log helps fixing the issue. IIRC the SDHCI driver
>> on VIM3 board uses internal SRAM for transferring data (instead of DMA),
>> so the issue is somehow related to that.
>
> Drivers shouldn't be using memcpy() on iomem mappings. Even if they 
> happen to have got away with it sometimes ;)
>
> Taking a quick look at that driver,
>
>     host->bounce_buf = host->regs + SD_EMMC_SRAM_DATA_BUF_OFF;
>
> is completely bogus, as Sparse will readily point out.
>
> Robin.
>
>>> ---
>>>    arch/arm64/lib/Makefile  |   2 +-
>>>    arch/arm64/lib/memcpy.S  | 272 
>>> ++++++++++++++++++++++++++++++++-------
>>>    arch/arm64/lib/memmove.S | 189 ---------------------------
>>>    3 files changed, 230 insertions(+), 233 deletions(-)
>>>    delete mode 100644 arch/arm64/lib/memmove.S
>>>
>>> diff --git a/arch/arm64/lib/Makefile b/arch/arm64/lib/Makefile
>>> index d31e1169d9b8..01c596aa539c 100644
>>> --- a/arch/arm64/lib/Makefile
>>> +++ b/arch/arm64/lib/Makefile
>>> @@ -1,7 +1,7 @@
>>>    # SPDX-License-Identifier: GPL-2.0
>>>    lib-y        := clear_user.o delay.o copy_from_user.o        \
>>>               copy_to_user.o copy_in_user.o copy_page.o \
>>> -           clear_page.o csum.o memchr.o memcpy.o memmove.o \
>>> +           clear_page.o csum.o memchr.o memcpy.o        \
>>>               memset.o memcmp.o strcmp.o strncmp.o strlen.o \
>>>               strnlen.o strchr.o strrchr.o tishift.o
>>>    diff --git a/arch/arm64/lib/memcpy.S b/arch/arm64/lib/memcpy.S
>>> index dc8d2a216a6e..31073a8304fb 100644
>>> --- a/arch/arm64/lib/memcpy.S
>>> +++ b/arch/arm64/lib/memcpy.S
>>> @@ -1,66 +1,252 @@
>>>    /* SPDX-License-Identifier: GPL-2.0-only */
>>>    /*
>>> - * Copyright (C) 2013 ARM Ltd.
>>> - * Copyright (C) 2013 Linaro.
>>> + * Copyright (c) 2012-2020, Arm Limited.
>>>     *
>>> - * This code is based on glibc cortex strings work originally 
>>> authored by Linaro
>>> - * be found @
>>> - *
>>> - * 
>>> http://bazaar.launchpad.net/~linaro-toolchain-dev/cortex-strings/trunk/
>>> - * files/head:/src/aarch64/
>>> + * Adapted from the original at:
>>> + * 
>>> https://protect2.fireeye.com/v1/url?k=30360c6b-6fad3573-30378724-0cc47a314e9a-eee98177cc643ca2&q=1&e=fcfaf71d-f01a-4bc4-8e16-8ae86e0c0116&u=https%3A%2F%2Fgithub.com%2FARM-software%2Foptimized-routines%2Fblob%2Fmaster%2Fstring%2Faarch64%2Fmemcpy.S
>>>     */
>>>       #include <linux/linkage.h>
>>>    #include <asm/assembler.h>
>>> -#include <asm/cache.h>
>>>    -/*
>>> - * Copy a buffer from src to dest (alignment handled by the hardware)
>>> +/* Assumptions:
>>> + *
>>> + * ARMv8-a, AArch64, unaligned accesses.
>>>     *
>>> - * Parameters:
>>> - *    x0 - dest
>>> - *    x1 - src
>>> - *    x2 - n
>>> - * Returns:
>>> - *    x0 - dest
>>>     */
>>> -    .macro ldrb1 reg, ptr, val
>>> -    ldrb  \reg, [\ptr], \val
>>> -    .endm
>>>    -    .macro strb1 reg, ptr, val
>>> -    strb \reg, [\ptr], \val
>>> -    .endm
>>> +#define L(label) .L ## label
>>>    -    .macro ldrh1 reg, ptr, val
>>> -    ldrh  \reg, [\ptr], \val
>>> -    .endm
>>> +#define dstin    x0
>>> +#define src    x1
>>> +#define count    x2
>>> +#define dst    x3
>>> +#define srcend    x4
>>> +#define dstend    x5
>>> +#define A_l    x6
>>> +#define A_lw    w6
>>> +#define A_h    x7
>>> +#define B_l    x8
>>> +#define B_lw    w8
>>> +#define B_h    x9
>>> +#define C_l    x10
>>> +#define C_lw    w10
>>> +#define C_h    x11
>>> +#define D_l    x12
>>> +#define D_h    x13
>>> +#define E_l    x14
>>> +#define E_h    x15
>>> +#define F_l    x16
>>> +#define F_h    x17
>>> +#define G_l    count
>>> +#define G_h    dst
>>> +#define H_l    src
>>> +#define H_h    srcend
>>> +#define tmp1    x14
>>>    -    .macro strh1 reg, ptr, val
>>> -    strh \reg, [\ptr], \val
>>> -    .endm
>>> +/* This implementation handles overlaps and supports both memcpy 
>>> and memmove
>>> +   from a single entry point.  It uses unaligned accesses and 
>>> branchless
>>> +   sequences to keep the code small, simple and improve performance.
>>>    -    .macro ldr1 reg, ptr, val
>>> -    ldr \reg, [\ptr], \val
>>> -    .endm
>>> +   Copies are split into 3 main cases: small copies of up to 32 
>>> bytes, medium
>>> +   copies of up to 128 bytes, and large copies.  The overhead of 
>>> the overlap
>>> +   check is negligible since it is only required for large copies.
>>>    -    .macro str1 reg, ptr, val
>>> -    str \reg, [\ptr], \val
>>> -    .endm
>>> -
>>> -    .macro ldp1 reg1, reg2, ptr, val
>>> -    ldp \reg1, \reg2, [\ptr], \val
>>> -    .endm
>>> -
>>> -    .macro stp1 reg1, reg2, ptr, val
>>> -    stp \reg1, \reg2, [\ptr], \val
>>> -    .endm
>>> +   Large copies use a software pipelined loop processing 64 bytes 
>>> per iteration.
>>> +   The destination pointer is 16-byte aligned to minimize unaligned 
>>> accesses.
>>> +   The loop tail is handled by always copying 64 bytes from the end.
>>> +*/
>>>    +SYM_FUNC_START_ALIAS(__memmove)
>>> +SYM_FUNC_START_WEAK_ALIAS_PI(memmove)
>>>    SYM_FUNC_START_ALIAS(__memcpy)
>>>    SYM_FUNC_START_WEAK_PI(memcpy)
>>> -#include "copy_template.S"
>>> +    add    srcend, src, count
>>> +    add    dstend, dstin, count
>>> +    cmp    count, 128
>>> +    b.hi    L(copy_long)
>>> +    cmp    count, 32
>>> +    b.hi    L(copy32_128)
>>> +
>>> +    /* Small copies: 0..32 bytes.  */
>>> +    cmp    count, 16
>>> +    b.lo    L(copy16)
>>> +    ldp    A_l, A_h, [src]
>>> +    ldp    D_l, D_h, [srcend, -16]
>>> +    stp    A_l, A_h, [dstin]
>>> +    stp    D_l, D_h, [dstend, -16]
>>>        ret
>>> +
>>> +    /* Copy 8-15 bytes.  */
>>> +L(copy16):
>>> +    tbz    count, 3, L(copy8)
>>> +    ldr    A_l, [src]
>>> +    ldr    A_h, [srcend, -8]
>>> +    str    A_l, [dstin]
>>> +    str    A_h, [dstend, -8]
>>> +    ret
>>> +
>>> +    .p2align 3
>>> +    /* Copy 4-7 bytes.  */
>>> +L(copy8):
>>> +    tbz    count, 2, L(copy4)
>>> +    ldr    A_lw, [src]
>>> +    ldr    B_lw, [srcend, -4]
>>> +    str    A_lw, [dstin]
>>> +    str    B_lw, [dstend, -4]
>>> +    ret
>>> +
>>> +    /* Copy 0..3 bytes using a branchless sequence.  */
>>> +L(copy4):
>>> +    cbz    count, L(copy0)
>>> +    lsr    tmp1, count, 1
>>> +    ldrb    A_lw, [src]
>>> +    ldrb    C_lw, [srcend, -1]
>>> +    ldrb    B_lw, [src, tmp1]
>>> +    strb    A_lw, [dstin]
>>> +    strb    B_lw, [dstin, tmp1]
>>> +    strb    C_lw, [dstend, -1]
>>> +L(copy0):
>>> +    ret
>>> +
>>> +    .p2align 4
>>> +    /* Medium copies: 33..128 bytes.  */
>>> +L(copy32_128):
>>> +    ldp    A_l, A_h, [src]
>>> +    ldp    B_l, B_h, [src, 16]
>>> +    ldp    C_l, C_h, [srcend, -32]
>>> +    ldp    D_l, D_h, [srcend, -16]
>>> +    cmp    count, 64
>>> +    b.hi    L(copy128)
>>> +    stp    A_l, A_h, [dstin]
>>> +    stp    B_l, B_h, [dstin, 16]
>>> +    stp    C_l, C_h, [dstend, -32]
>>> +    stp    D_l, D_h, [dstend, -16]
>>> +    ret
>>> +
>>> +    .p2align 4
>>> +    /* Copy 65..128 bytes.  */
>>> +L(copy128):
>>> +    ldp    E_l, E_h, [src, 32]
>>> +    ldp    F_l, F_h, [src, 48]
>>> +    cmp    count, 96
>>> +    b.ls    L(copy96)
>>> +    ldp    G_l, G_h, [srcend, -64]
>>> +    ldp    H_l, H_h, [srcend, -48]
>>> +    stp    G_l, G_h, [dstend, -64]
>>> +    stp    H_l, H_h, [dstend, -48]
>>> +L(copy96):
>>> +    stp    A_l, A_h, [dstin]
>>> +    stp    B_l, B_h, [dstin, 16]
>>> +    stp    E_l, E_h, [dstin, 32]
>>> +    stp    F_l, F_h, [dstin, 48]
>>> +    stp    C_l, C_h, [dstend, -32]
>>> +    stp    D_l, D_h, [dstend, -16]
>>> +    ret
>>> +
>>> +    .p2align 4
>>> +    /* Copy more than 128 bytes.  */
>>> +L(copy_long):
>>> +    /* Use backwards copy if there is an overlap.  */
>>> +    sub    tmp1, dstin, src
>>> +    cbz    tmp1, L(copy0)
>>> +    cmp    tmp1, count
>>> +    b.lo    L(copy_long_backwards)
>>> +
>>> +    /* Copy 16 bytes and then align dst to 16-byte alignment.  */
>>> +
>>> +    ldp    D_l, D_h, [src]
>>> +    and    tmp1, dstin, 15
>>> +    bic    dst, dstin, 15
>>> +    sub    src, src, tmp1
>>> +    add    count, count, tmp1    /* Count is now 16 too large.  */
>>> +    ldp    A_l, A_h, [src, 16]
>>> +    stp    D_l, D_h, [dstin]
>>> +    ldp    B_l, B_h, [src, 32]
>>> +    ldp    C_l, C_h, [src, 48]
>>> +    ldp    D_l, D_h, [src, 64]!
>>> +    subs    count, count, 128 + 16    /* Test and readjust count.  */
>>> +    b.ls    L(copy64_from_end)
>>> +
>>> +L(loop64):
>>> +    stp    A_l, A_h, [dst, 16]
>>> +    ldp    A_l, A_h, [src, 16]
>>> +    stp    B_l, B_h, [dst, 32]
>>> +    ldp    B_l, B_h, [src, 32]
>>> +    stp    C_l, C_h, [dst, 48]
>>> +    ldp    C_l, C_h, [src, 48]
>>> +    stp    D_l, D_h, [dst, 64]!
>>> +    ldp    D_l, D_h, [src, 64]!
>>> +    subs    count, count, 64
>>> +    b.hi    L(loop64)
>>> +
>>> +    /* Write the last iteration and copy 64 bytes from the end.  */
>>> +L(copy64_from_end):
>>> +    ldp    E_l, E_h, [srcend, -64]
>>> +    stp    A_l, A_h, [dst, 16]
>>> +    ldp    A_l, A_h, [srcend, -48]
>>> +    stp    B_l, B_h, [dst, 32]
>>> +    ldp    B_l, B_h, [srcend, -32]
>>> +    stp    C_l, C_h, [dst, 48]
>>> +    ldp    C_l, C_h, [srcend, -16]
>>> +    stp    D_l, D_h, [dst, 64]
>>> +    stp    E_l, E_h, [dstend, -64]
>>> +    stp    A_l, A_h, [dstend, -48]
>>> +    stp    B_l, B_h, [dstend, -32]
>>> +    stp    C_l, C_h, [dstend, -16]
>>> +    ret
>>> +
>>> +    .p2align 4
>>> +
>>> +    /* Large backwards copy for overlapping copies.
>>> +       Copy 16 bytes and then align dst to 16-byte alignment.  */
>>> +L(copy_long_backwards):
>>> +    ldp    D_l, D_h, [srcend, -16]
>>> +    and    tmp1, dstend, 15
>>> +    sub    srcend, srcend, tmp1
>>> +    sub    count, count, tmp1
>>> +    ldp    A_l, A_h, [srcend, -16]
>>> +    stp    D_l, D_h, [dstend, -16]
>>> +    ldp    B_l, B_h, [srcend, -32]
>>> +    ldp    C_l, C_h, [srcend, -48]
>>> +    ldp    D_l, D_h, [srcend, -64]!
>>> +    sub    dstend, dstend, tmp1
>>> +    subs    count, count, 128
>>> +    b.ls    L(copy64_from_start)
>>> +
>>> +L(loop64_backwards):
>>> +    stp    A_l, A_h, [dstend, -16]
>>> +    ldp    A_l, A_h, [srcend, -16]
>>> +    stp    B_l, B_h, [dstend, -32]
>>> +    ldp    B_l, B_h, [srcend, -32]
>>> +    stp    C_l, C_h, [dstend, -48]
>>> +    ldp    C_l, C_h, [srcend, -48]
>>> +    stp    D_l, D_h, [dstend, -64]!
>>> +    ldp    D_l, D_h, [srcend, -64]!
>>> +    subs    count, count, 64
>>> +    b.hi    L(loop64_backwards)
>>> +
>>> +    /* Write the last iteration and copy 64 bytes from the start.  */
>>> +L(copy64_from_start):
>>> +    ldp    G_l, G_h, [src, 48]
>>> +    stp    A_l, A_h, [dstend, -16]
>>> +    ldp    A_l, A_h, [src, 32]
>>> +    stp    B_l, B_h, [dstend, -32]
>>> +    ldp    B_l, B_h, [src, 16]
>>> +    stp    C_l, C_h, [dstend, -48]
>>> +    ldp    C_l, C_h, [src]
>>> +    stp    D_l, D_h, [dstend, -64]
>>> +    stp    G_l, G_h, [dstin, 48]
>>> +    stp    A_l, A_h, [dstin, 32]
>>> +    stp    B_l, B_h, [dstin, 16]
>>> +    stp    C_l, C_h, [dstin]
>>> +    ret
>>> +
>>>    SYM_FUNC_END_PI(memcpy)
>>>    EXPORT_SYMBOL(memcpy)
>>>    SYM_FUNC_END_ALIAS(__memcpy)
>>>    EXPORT_SYMBOL(__memcpy)
>>> +SYM_FUNC_END_ALIAS_PI(memmove)
>>> +EXPORT_SYMBOL(memmove)
>>> +SYM_FUNC_END_ALIAS(__memmove)
>>> +EXPORT_SYMBOL(__memmove)
>>> \ No newline at end of file
>>> diff --git a/arch/arm64/lib/memmove.S b/arch/arm64/lib/memmove.S
>>> deleted file mode 100644
>>> index 1035dce4bdaf..000000000000
>>> --- a/arch/arm64/lib/memmove.S
>>> +++ /dev/null
>>> @@ -1,189 +0,0 @@
>>> -/* SPDX-License-Identifier: GPL-2.0-only */
>>> -/*
>>> - * Copyright (C) 2013 ARM Ltd.
>>> - * Copyright (C) 2013 Linaro.
>>> - *
>>> - * This code is based on glibc cortex strings work originally 
>>> authored by Linaro
>>> - * be found @
>>> - *
>>> - * 
>>> http://bazaar.launchpad.net/~linaro-toolchain-dev/cortex-strings/trunk/
>>> - * files/head:/src/aarch64/
>>> - */
>>> -
>>> -#include <linux/linkage.h>
>>> -#include <asm/assembler.h>
>>> -#include <asm/cache.h>
>>> -
>>> -/*
>>> - * Move a buffer from src to test (alignment handled by the hardware).
>>> - * If dest <= src, call memcpy, otherwise copy in reverse order.
>>> - *
>>> - * Parameters:
>>> - *    x0 - dest
>>> - *    x1 - src
>>> - *    x2 - n
>>> - * Returns:
>>> - *    x0 - dest
>>> - */
>>> -dstin    .req    x0
>>> -src    .req    x1
>>> -count    .req    x2
>>> -tmp1    .req    x3
>>> -tmp1w    .req    w3
>>> -tmp2    .req    x4
>>> -tmp2w    .req    w4
>>> -tmp3    .req    x5
>>> -tmp3w    .req    w5
>>> -dst    .req    x6
>>> -
>>> -A_l    .req    x7
>>> -A_h    .req    x8
>>> -B_l    .req    x9
>>> -B_h    .req    x10
>>> -C_l    .req    x11
>>> -C_h    .req    x12
>>> -D_l    .req    x13
>>> -D_h    .req    x14
>>> -
>>> -SYM_FUNC_START_ALIAS(__memmove)
>>> -SYM_FUNC_START_WEAK_PI(memmove)
>>> -    cmp    dstin, src
>>> -    b.lo    __memcpy
>>> -    add    tmp1, src, count
>>> -    cmp    dstin, tmp1
>>> -    b.hs    __memcpy        /* No overlap.  */
>>> -
>>> -    add    dst, dstin, count
>>> -    add    src, src, count
>>> -    cmp    count, #16
>>> -    b.lo    .Ltail15  /*probably non-alignment accesses.*/
>>> -
>>> -    ands    tmp2, src, #15     /* Bytes to reach alignment. */
>>> -    b.eq    .LSrcAligned
>>> -    sub    count, count, tmp2
>>> -    /*
>>> -    * process the aligned offset length to make the src aligned 
>>> firstly.
>>> -    * those extra instructions' cost is acceptable. It also make the
>>> -    * coming accesses are based on aligned address.
>>> -    */
>>> -    tbz    tmp2, #0, 1f
>>> -    ldrb    tmp1w, [src, #-1]!
>>> -    strb    tmp1w, [dst, #-1]!
>>> -1:
>>> -    tbz    tmp2, #1, 2f
>>> -    ldrh    tmp1w, [src, #-2]!
>>> -    strh    tmp1w, [dst, #-2]!
>>> -2:
>>> -    tbz    tmp2, #2, 3f
>>> -    ldr    tmp1w, [src, #-4]!
>>> -    str    tmp1w, [dst, #-4]!
>>> -3:
>>> -    tbz    tmp2, #3, .LSrcAligned
>>> -    ldr    tmp1, [src, #-8]!
>>> -    str    tmp1, [dst, #-8]!
>>> -
>>> -.LSrcAligned:
>>> -    cmp    count, #64
>>> -    b.ge    .Lcpy_over64
>>> -
>>> -    /*
>>> -    * Deal with small copies quickly by dropping straight into the
>>> -    * exit block.
>>> -    */
>>> -.Ltail63:
>>> -    /*
>>> -    * Copy up to 48 bytes of data. At this point we only need the
>>> -    * bottom 6 bits of count to be accurate.
>>> -    */
>>> -    ands    tmp1, count, #0x30
>>> -    b.eq    .Ltail15
>>> -    cmp    tmp1w, #0x20
>>> -    b.eq    1f
>>> -    b.lt    2f
>>> -    ldp    A_l, A_h, [src, #-16]!
>>> -    stp    A_l, A_h, [dst, #-16]!
>>> -1:
>>> -    ldp    A_l, A_h, [src, #-16]!
>>> -    stp    A_l, A_h, [dst, #-16]!
>>> -2:
>>> -    ldp    A_l, A_h, [src, #-16]!
>>> -    stp    A_l, A_h, [dst, #-16]!
>>> -
>>> -.Ltail15:
>>> -    tbz    count, #3, 1f
>>> -    ldr    tmp1, [src, #-8]!
>>> -    str    tmp1, [dst, #-8]!
>>> -1:
>>> -    tbz    count, #2, 2f
>>> -    ldr    tmp1w, [src, #-4]!
>>> -    str    tmp1w, [dst, #-4]!
>>> -2:
>>> -    tbz    count, #1, 3f
>>> -    ldrh    tmp1w, [src, #-2]!
>>> -    strh    tmp1w, [dst, #-2]!
>>> -3:
>>> -    tbz    count, #0, .Lexitfunc
>>> -    ldrb    tmp1w, [src, #-1]
>>> -    strb    tmp1w, [dst, #-1]
>>> -
>>> -.Lexitfunc:
>>> -    ret
>>> -
>>> -.Lcpy_over64:
>>> -    subs    count, count, #128
>>> -    b.ge    .Lcpy_body_large
>>> -    /*
>>> -    * Less than 128 bytes to copy, so handle 64 bytes here and then 
>>> jump
>>> -    * to the tail.
>>> -    */
>>> -    ldp    A_l, A_h, [src, #-16]
>>> -    stp    A_l, A_h, [dst, #-16]
>>> -    ldp    B_l, B_h, [src, #-32]
>>> -    ldp    C_l, C_h, [src, #-48]
>>> -    stp    B_l, B_h, [dst, #-32]
>>> -    stp    C_l, C_h, [dst, #-48]
>>> -    ldp    D_l, D_h, [src, #-64]!
>>> -    stp    D_l, D_h, [dst, #-64]!
>>> -
>>> -    tst    count, #0x3f
>>> -    b.ne    .Ltail63
>>> -    ret
>>> -
>>> -    /*
>>> -    * Critical loop. Start at a new cache line boundary. Assuming
>>> -    * 64 bytes per line this ensures the entire loop is in one line.
>>> -    */
>>> -    .p2align    L1_CACHE_SHIFT
>>> -.Lcpy_body_large:
>>> -    /* pre-load 64 bytes data. */
>>> -    ldp    A_l, A_h, [src, #-16]
>>> -    ldp    B_l, B_h, [src, #-32]
>>> -    ldp    C_l, C_h, [src, #-48]
>>> -    ldp    D_l, D_h, [src, #-64]!
>>> -1:
>>> -    /*
>>> -    * interlace the load of next 64 bytes data block with store of 
>>> the last
>>> -    * loaded 64 bytes data.
>>> -    */
>>> -    stp    A_l, A_h, [dst, #-16]
>>> -    ldp    A_l, A_h, [src, #-16]
>>> -    stp    B_l, B_h, [dst, #-32]
>>> -    ldp    B_l, B_h, [src, #-32]
>>> -    stp    C_l, C_h, [dst, #-48]
>>> -    ldp    C_l, C_h, [src, #-48]
>>> -    stp    D_l, D_h, [dst, #-64]!
>>> -    ldp    D_l, D_h, [src, #-64]!
>>> -    subs    count, count, #64
>>> -    b.ge    1b
>>> -    stp    A_l, A_h, [dst, #-16]
>>> -    stp    B_l, B_h, [dst, #-32]
>>> -    stp    C_l, C_h, [dst, #-48]
>>> -    stp    D_l, D_h, [dst, #-64]!
>>> -
>>> -    tst    count, #0x3f
>>> -    b.ne    .Ltail63
>>> -    ret
>>> -SYM_FUNC_END_PI(memmove)
>>> -EXPORT_SYMBOL(memmove)
>>> -SYM_FUNC_END_ALIAS(__memmove)
>>> -EXPORT_SYMBOL(__memmove)
>>
>> Best regards
>>
>
Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland


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

Re: [PATCH v2 6/8] arm64: Import latest memcpy()/memmove() implementation

[Neil Armstrong]

Hi,

On 08/06/2021 14:21, Marek Szyprowski wrote:
> + Kevin
> 
> On 08.06.2021 13:37, Robin Murphy wrote:
>> Hi Marek,
>>
>> On 2021-06-08 12:15, Marek Szyprowski wrote:
>>> Hi Robin,
>>>
>>> On 27.05.2021 17:34, Robin Murphy wrote:
>>>> Import the latest implementation of memcpy(), based on the
>>>> upstream code of string/aarch64/memcpy.S at commit afd6244 from
>>>> https://protect2.fireeye.com/v1/url?k=0e25d630-51beef28-0e245d7f-0cc47a314e9a-b41fdb2d4d06ff75&q=1&e=fcfaf71d-f01a-4bc4-8e16-8ae86e0c0116&u=https%3A%2F%2Fgithub.com%2FARM-software%2Foptimized-routines, 
>>>> and subsuming
>>>> memmove() in the process.
>>>>
>>>> Note that for simplicity Arm have chosen to contribute this code
>>>> to Linux under GPLv2 rather than the original MIT license.
>>>>
>>>> Note also that the needs of the usercopy routines vs. regular memcpy()
>>>> have now diverged so far that we abandon the shared template idea
>>>> and the damage which that incurred to the tuning of LDP/STP loops.
>>>> We'll be back to tackle those routines separately in future.
>>>>
>>>> Signed-off-by: Robin Murphy <robin.murphy@arm.com>
>>>
>>> This patch landed recently in linux-next as commit 285133040e6c ("arm64:
>>> Import latest memcpy()/memmove() implementation"). Sadly it causes
>>> serious issues on Khadas VIM3 board. Reverting it on top of linux
>>> next-20210607 (together with 6b8f648959e5 and resolving the conflict in
>>> the Makefile) fixes the issue. Here is the kernel log:
>>>
>>> Unable to handle kernel paging request at virtual address 
>>> ffff8000136bd204
>>> Mem abort info:
>>>     ESR = 0x96000061
>>>     EC = 0x25: DABT (current EL), IL = 32 bits
>>>     SET = 0, FnV = 0
>>>     EA = 0, S1PTW = 0
>>> Data abort info:
>>>     ISV = 0, ISS = 0x00000061
>>
>> That's an alignment fault, which implies we're accessing something 
>> which isn't normal memory.
>>
>>>     CM = 0, WnR = 1
>>> swapper pgtable: 4k pages, 48-bit VAs, pgdp=0000000009da6000
>>> [ffff8000136bd204] pgd=10000000f4806003, p4d=10000000f4806003,
>>> pud=10000000f4805003, pmd=1000000000365003, pte=00680000ffe03713
>>> Internal error: Oops: 96000061 [#1] PREEMPT SMP
>>> Modules linked in: brcmfmac brcmutil cfg80211 dw_hdmi_i2s_audio
>>> meson_gxl hci_uart btqca btbcm bluetooth panfrost ecdh_generic ecc
>>> snd_soc_meson_axg_sound_card crct10dif_ce snd_soc_meson_card_utils
>>> rfkill rtc_hym8563 gpu_sched dwmac_generic rc_khadas meson_gxbb_wdt
>>> meson_ir pwm_meson snd_soc_meson_axg_tdmin snd_soc_meson_g12a_tohdmitx
>>> rtc_meson_vrtc snd_soc_meson_axg_tdmout snd_soc_meson_axg_frddr
>>> reset_meson_audio_arb snd_soc_meson_codec_glue axg_audio meson_rng
>>> sclk_div dwmac_meson8b snd_soc_meson_axg_toddr mdio_mux_meson_g12a
>>> clk_phase stmmac_platform rng_core snd_soc_meson_axg_fifo meson_dw_hdmi
>>> stmmac meson_drm meson_canvas dw_hdmi pcs_xpcs display_connector
>>> snd_soc_meson_axg_tdm_interface nvmem_meson_efuse adc_keys
>>> snd_soc_meson_axg_tdm_formatter
>>> CPU: 4 PID: 135 Comm: kworker/4:3 Not tainted 5.13.0-rc5-next-20210607
>>> #10441
>>> Hardware name: Khadas VIM3 (DT)
>>> Workqueue: events request_firmware_work_func
>>> pstate: 20000005 (nzCv daif -PAN -UAO -TCO BTYPE=--)
>>> pc : __memcpy+0x2c/0x260
>>> lr : sg_copy_buffer+0x90/0x118
>>> ...
>>> Call trace:
>>>    __memcpy+0x2c/0x260
>>>    sg_copy_to_buffer+0x14/0x20
>>>    meson_mmc_start_cmd+0xf4/0x2c8
>>>    meson_mmc_request+0x4c/0xb8
>>>    __mmc_start_request+0xa4/0x2a8
>>>    mmc_start_request+0x80/0xa8
>>>    mmc_wait_for_req+0x68/0xd8
>>>    mmc_io_rw_extended+0x1d4/0x2e0
>>>    sdio_io_rw_ext_helper+0xb0/0x1e8
>>>    sdio_memcpy_toio+0x20/0x28
>>>    brcmf_sdiod_skbuff_write.isra.18+0x2c/0x68 [brcmfmac]
>>>    brcmf_sdiod_ramrw+0xe0/0x230 [brcmfmac]
>>>    brcmf_sdio_firmware_callback+0xa8/0x7c8 [brcmfmac]
>>>    brcmf_fw_request_done+0x7c/0x100 [brcmfmac]
>>>    request_firmware_work_func+0x4c/0xd8
>>>    process_one_work+0x2a8/0x718
>>>    worker_thread+0x48/0x460
>>>    kthread+0x12c/0x160
>>>    ret_from_fork+0x10/0x18
>>> Code: 540000c3 a9401c26 a97f348c a9001c06 (a93f34ac)
>>> ---[ end trace be83fa283dc82415 ]---
>>>
>>> I hope that the above log helps fixing the issue. IIRC the SDHCI driver
>>> on VIM3 board uses internal SRAM for transferring data (instead of DMA),
>>> so the issue is somehow related to that.
>>
>> Drivers shouldn't be using memcpy() on iomem mappings. Even if they 
>> happen to have got away with it sometimes ;)
>>
>> Taking a quick look at that driver,
>>
>>     host->bounce_buf = host->regs + SD_EMMC_SRAM_DATA_BUF_OFF;
>>
>> is completely bogus, as Sparse will readily point out.

My bad, what's the correct way to copy data to an iomem mapping ?

Neil

>>
>> Robin.
>>

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

Re: [PATCH v2 6/8] arm64: Import latest memcpy()/memmove() implementation

[Mark Rutland]

On Tue, Jun 08, 2021 at 02:36:26PM +0200, Neil Armstrong wrote:
> On 08/06/2021 14:21, Marek Szyprowski wrote:
> > On 08.06.2021 13:37, Robin Murphy wrote:
> >> On 2021-06-08 12:15, Marek Szyprowski wrote:
> >>> This patch landed recently in linux-next as commit 285133040e6c ("arm64:
> >>> Import latest memcpy()/memmove() implementation"). Sadly it causes
> >>> serious issues on Khadas VIM3 board. Reverting it on top of linux
> >>> next-20210607 (together with 6b8f648959e5 and resolving the conflict in
> >>> the Makefile) fixes the issue. Here is the kernel log:
> >>>
> >>> Unable to handle kernel paging request at virtual address 
> >>> ffff8000136bd204
> >>> Mem abort info:
> >>>     ESR = 0x96000061
> >>>     EC = 0x25: DABT (current EL), IL = 32 bits
> >>>     SET = 0, FnV = 0
> >>>     EA = 0, S1PTW = 0
> >>> Data abort info:
> >>>     ISV = 0, ISS = 0x00000061
> >>
> >> That's an alignment fault, which implies we're accessing something 
> >> which isn't normal memory.

[...]

> >>> I hope that the above log helps fixing the issue. IIRC the SDHCI driver
> >>> on VIM3 board uses internal SRAM for transferring data (instead of DMA),
> >>> so the issue is somehow related to that.
> >>
> >> Drivers shouldn't be using memcpy() on iomem mappings. Even if they 
> >> happen to have got away with it sometimes ;)
> >>
> >> Taking a quick look at that driver,
> >>
> >>     host->bounce_buf = host->regs + SD_EMMC_SRAM_DATA_BUF_OFF;
> >>
> >> is completely bogus, as Sparse will readily point out.
> 
> My bad, what's the correct way to copy data to an iomem mapping ?

We have memcpy_toio() and memcpy_fromio() for this.

Thanks,
Mark.

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

Re: [PATCH v2 6/8] arm64: Import latest memcpy()/memmove() implementation

[dann frazier]

On Tue, Jun 08, 2021 at 01:42:48PM +0100, Mark Rutland wrote:
> On Tue, Jun 08, 2021 at 02:36:26PM +0200, Neil Armstrong wrote:
> > On 08/06/2021 14:21, Marek Szyprowski wrote:
> > > On 08.06.2021 13:37, Robin Murphy wrote:
> > >> On 2021-06-08 12:15, Marek Szyprowski wrote:
> > >>> This patch landed recently in linux-next as commit 285133040e6c ("arm64:
> > >>> Import latest memcpy()/memmove() implementation"). Sadly it causes
> > >>> serious issues on Khadas VIM3 board. Reverting it on top of linux
> > >>> next-20210607 (together with 6b8f648959e5 and resolving the conflict in
> > >>> the Makefile) fixes the issue. Here is the kernel log:
> > >>>
> > >>> Unable to handle kernel paging request at virtual address 
> > >>> ffff8000136bd204
> > >>> Mem abort info:
> > >>>     ESR = 0x96000061
> > >>>     EC = 0x25: DABT (current EL), IL = 32 bits
> > >>>     SET = 0, FnV = 0
> > >>>     EA = 0, S1PTW = 0
> > >>> Data abort info:
> > >>>     ISV = 0, ISS = 0x00000061
> > >>
> > >> That's an alignment fault, which implies we're accessing something 
> > >> which isn't normal memory.
> 
> [...]
> 
> > >>> I hope that the above log helps fixing the issue. IIRC the SDHCI driver
> > >>> on VIM3 board uses internal SRAM for transferring data (instead of DMA),
> > >>> so the issue is somehow related to that.
> > >>
> > >> Drivers shouldn't be using memcpy() on iomem mappings. Even if they 
> > >> happen to have got away with it sometimes ;)
> > >>
> > >> Taking a quick look at that driver,
> > >>
> > >>     host->bounce_buf = host->regs + SD_EMMC_SRAM_DATA_BUF_OFF;
> > >>
> > >> is completely bogus, as Sparse will readily point out.
> > 
> > My bad, what's the correct way to copy data to an iomem mapping ?
> 
> We have memcpy_toio() and memcpy_fromio() for this.

ltp's read_all_sys test is triggering something similar which I
bisected back to this commit - see below. Does this imply we need
something like a memory_read_from_*io*_buffer()?

[ 2583.023514] Unable to handle kernel paging request at virtual address ffff80004a3003bf
[ 2583.031456] Mem abort info:
[ 2583.034259]   ESR = 0x96000021
[ 2583.037317]   EC = 0x25: DABT (current EL), IL = 32 bits
[ 2583.042632]   SET = 0, FnV = 0
[ 2583.045689]   EA = 0, S1PTW = 0
[ 2583.048834] Data abort info:
[ 2583.051704]   ISV = 0, ISS = 0x00000021
[ 2583.055542]   CM = 0, WnR = 0
[ 2583.058512] swapper pgtable: 4k pages, 48-bit VAs, pgdp=0000401182231000
[ 2583.065217] [ffff80004a3003bf] pgd=10000800001a2003, p4d=10000800001a2003, pud=100008000fa35003, pmd=100008001ddbd003, pte=0068000088230f13
[ 2583.077751] Internal error: Oops: 96000021 [#22] SMP
[ 2583.082710] Modules linked in: nls_iso8859_1 joydev input_leds efi_pstore arm_spe_pmu acpi_ipmi ipmi_ssif arm_cmn xgene_hwmon arm_dmc620_pmu arm_dsu_pmu cppc_cpufreq acpi_tad sch_fq_codel dm_multipath scsi_dh_rdac scsi_dh_emc scsi_dh_alua ipmi_devintf ipmi_msghandler ip_tables x_tables autofs4 btrfs blake2b_generic zstd_compress raid10 raid456 async_raid6_recov async_memcpy async_pq async_xor async_tx xor xor_neon raid6_pq libcrc32c raid1 raid0 multipath linear hid_generic usbhid cdc_ether hid usbnet mlx5_ib ib_uverbs ib_core uas usb_storage ast drm_vram_helper drm_ttm_helper ttm i2c_algo_bit drm_kms_helper syscopyarea crct10dif_ce sysfillrect ghash_ce sysimgblt sha2_ce fb_sys_fops sha256_arm64 cec sha1_ce rc_core mlx5_core nvme drm psample xhci_pci nvme_core mlxfw xhci_pci_renesas tls aes_neon_bs aes_neon_blk aes_ce_blk crypto_simd cryptd aes_ce_cipher
[ 2583.158313] CPU: 38 PID: 8392 Comm: read_all Tainted: G      D           5.13.0-rc3+ #15
[ 2583.166394] Hardware name: WIWYNN Mt.Jade Server System B81.030Z1.0007/Mt.Jade Motherboard, BIOS 1.6.20210526 (SCP: 1.06.20210526) 2021/05/26
[ 2583.179075] pstate: 80400009 (Nzcv daif +PAN -UAO -TCO BTYPE=--)
[ 2583.185072] pc : __memcpy+0x168/0x260
[ 2583.188735] lr : memory_read_from_buffer+0x58/0x80
[ 2583.193524] sp : ffff80004321bb40
[ 2583.196826] x29: ffff80004321bb40 x28: ffff07ffdd8bae80 x27: 0000000000000000
[ 2583.203952] x26: 0000000000000000 x25: 0000000000000000 x24: ffff07ffd2bd5820
[ 2583.211077] x23: ffff80004321bc30 x22: 00000000000003ff x21: ffff80004321bba8
[ 2583.218201] x20: 00000000000003ff x19: 00000000000003ff x18: 0000000000000000
[ 2583.225326] x17: 0000000000000000 x16: 0000000000000000 x15: 0000000000000000
[ 2583.232449] x14: 0000000000000000 x13: 0000000000000000 x12: 0000000000000000
[ 2583.239573] x11: 0000000000000000 x10: 0000000000000000 x9 : 0000000000000000
[ 2583.246697] x8 : 0000000000000000 x7 : 0000000000000000 x6 : 0000000000000000
[ 2583.253820] x5 : ffff07ffb7d93bff x4 : ffff80004a3003ff x3 : ffff07ffb7d93b80
[ 2583.260945] x2 : ffffffffffffffef x1 : ffff80004a3003c0 x0 : ffff07ffb7d93800
[ 2583.268069] Call trace:
[ 2583.270504]  __memcpy+0x168/0x260
[ 2583.273807]  acpi_data_show+0x5c/0x8c
[ 2583.277464]  sysfs_kf_bin_read+0x78/0xa0
[ 2583.281378]  kernfs_fop_read_iter+0xac/0x1e0
[ 2583.285637]  new_sync_read+0xf0/0x184
[ 2583.289290]  vfs_read+0x158/0x1e4
[ 2583.292594]  ksys_read+0x74/0x100
[ 2583.295898]  __arm64_sys_read+0x28/0x34
[ 2583.299723]  invoke_syscall+0x78/0x100
[ 2583.303466]  el0_svc_common.constprop.0+0x158/0x160
[ 2583.308332]  do_el0_svc+0x34/0xa0
[ 2583.311637]  el0_svc+0x2c/0x54
[ 2583.314685]  el0_sync_handler+0xa4/0x130
[ 2583.318596]  el0_sync+0x19c/0x1c0
[ 2583.321903] Code: a984346c a9c4342c f1010042 54fffee8 (a97c3c8e) 
[ 2583.327989] ---[ end trace e19a85d1dd8510e5 ]---

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

Re: [PATCH v2 6/8] arm64: Import latest memcpy()/memmove() implementation

[Robin Murphy]

On 2022-05-21 00:30, dann frazier wrote:
> On Tue, Jun 08, 2021 at 01:42:48PM +0100, Mark Rutland wrote:
>> On Tue, Jun 08, 2021 at 02:36:26PM +0200, Neil Armstrong wrote:
>>> On 08/06/2021 14:21, Marek Szyprowski wrote:
>>>> On 08.06.2021 13:37, Robin Murphy wrote:
>>>>> On 2021-06-08 12:15, Marek Szyprowski wrote:
>>>>>> This patch landed recently in linux-next as commit 285133040e6c ("arm64:
>>>>>> Import latest memcpy()/memmove() implementation"). Sadly it causes
>>>>>> serious issues on Khadas VIM3 board. Reverting it on top of linux
>>>>>> next-20210607 (together with 6b8f648959e5 and resolving the conflict in
>>>>>> the Makefile) fixes the issue. Here is the kernel log:
>>>>>>
>>>>>> Unable to handle kernel paging request at virtual address
>>>>>> ffff8000136bd204
>>>>>> Mem abort info:
>>>>>> � � ESR = 0x96000061
>>>>>> � � EC = 0x25: DABT (current EL), IL = 32 bits
>>>>>> � � SET = 0, FnV = 0
>>>>>> � � EA = 0, S1PTW = 0
>>>>>> Data abort info:
>>>>>> � � ISV = 0, ISS = 0x00000061
>>>>>
>>>>> That's an alignment fault, which implies we're accessing something
>>>>> which isn't normal memory.
>>
>> [...]
>>
>>>>>> I hope that the above log helps fixing the issue. IIRC the SDHCI driver
>>>>>> on VIM3 board uses internal SRAM for transferring data (instead of DMA),
>>>>>> so the issue is somehow related to that.
>>>>>
>>>>> Drivers shouldn't be using memcpy() on iomem mappings. Even if they
>>>>> happen to have got away with it sometimes ;)
>>>>>
>>>>> Taking a quick look at that driver,
>>>>>
>>>>> ����host->bounce_buf = host->regs + SD_EMMC_SRAM_DATA_BUF_OFF;
>>>>>
>>>>> is completely bogus, as Sparse will readily point out.
>>>
>>> My bad, what's the correct way to copy data to an iomem mapping ?
>>
>> We have memcpy_toio() and memcpy_fromio() for this.
> 
> ltp's read_all_sys test is triggering something similar which I
> bisected back to this commit - see below. Does this imply we need
> something like a memory_read_from_*io*_buffer()?

Should be fixed soon:

https://lore.kernel.org/lkml/20220407105120.1280-1-lorenzo.pieralisi@arm.com/

Thanks,
Robin.

> [ 2583.023514] Unable to handle kernel paging request at virtual address ffff80004a3003bf
> [ 2583.031456] Mem abort info:
> [ 2583.034259]   ESR = 0x96000021
> [ 2583.037317]   EC = 0x25: DABT (current EL), IL = 32 bits
> [ 2583.042632]   SET = 0, FnV = 0
> [ 2583.045689]   EA = 0, S1PTW = 0
> [ 2583.048834] Data abort info:
> [ 2583.051704]   ISV = 0, ISS = 0x00000021
> [ 2583.055542]   CM = 0, WnR = 0
> [ 2583.058512] swapper pgtable: 4k pages, 48-bit VAs, pgdp=0000401182231000
> [ 2583.065217] [ffff80004a3003bf] pgd=10000800001a2003, p4d=10000800001a2003, pud=100008000fa35003, pmd=100008001ddbd003, pte=0068000088230f13
> [ 2583.077751] Internal error: Oops: 96000021 [#22] SMP
> [ 2583.082710] Modules linked in: nls_iso8859_1 joydev input_leds efi_pstore arm_spe_pmu acpi_ipmi ipmi_ssif arm_cmn xgene_hwmon arm_dmc620_pmu arm_dsu_pmu cppc_cpufreq acpi_tad sch_fq_codel dm_multipath scsi_dh_rdac scsi_dh_emc scsi_dh_alua ipmi_devintf ipmi_msghandler ip_tables x_tables autofs4 btrfs blake2b_generic zstd_compress raid10 raid456 async_raid6_recov async_memcpy async_pq async_xor async_tx xor xor_neon raid6_pq libcrc32c raid1 raid0 multipath linear hid_generic usbhid cdc_ether hid usbnet mlx5_ib ib_uverbs ib_core uas usb_storage ast drm_vram_helper drm_ttm_helper ttm i2c_algo_bit drm_kms_helper syscopyarea crct10dif_ce sysfillrect ghash_ce sysimgblt sha2_ce fb_sys_fops sha256_arm64 cec sha1_ce rc_core mlx5_core nvme drm psample xhci_pci nvme_core mlxfw xhci_pci_renesas tls aes_neon_bs aes_neon_blk aes_ce_blk crypto_simd cryptd aes_ce_cipher
> [ 2583.158313] CPU: 38 PID: 8392 Comm: read_all Tainted: G      D           5.13.0-rc3+ #15
> [ 2583.166394] Hardware name: WIWYNN Mt.Jade Server System B81.030Z1.0007/Mt.Jade Motherboard, BIOS 1.6.20210526 (SCP: 1.06.20210526) 2021/05/26
> [ 2583.179075] pstate: 80400009 (Nzcv daif +PAN -UAO -TCO BTYPE=--)
> [ 2583.185072] pc : __memcpy+0x168/0x260
> [ 2583.188735] lr : memory_read_from_buffer+0x58/0x80
> [ 2583.193524] sp : ffff80004321bb40
> [ 2583.196826] x29: ffff80004321bb40 x28: ffff07ffdd8bae80 x27: 0000000000000000
> [ 2583.203952] x26: 0000000000000000 x25: 0000000000000000 x24: ffff07ffd2bd5820
> [ 2583.211077] x23: ffff80004321bc30 x22: 00000000000003ff x21: ffff80004321bba8
> [ 2583.218201] x20: 00000000000003ff x19: 00000000000003ff x18: 0000000000000000
> [ 2583.225326] x17: 0000000000000000 x16: 0000000000000000 x15: 0000000000000000
> [ 2583.232449] x14: 0000000000000000 x13: 0000000000000000 x12: 0000000000000000
> [ 2583.239573] x11: 0000000000000000 x10: 0000000000000000 x9 : 0000000000000000
> [ 2583.246697] x8 : 0000000000000000 x7 : 0000000000000000 x6 : 0000000000000000
> [ 2583.253820] x5 : ffff07ffb7d93bff x4 : ffff80004a3003ff x3 : ffff07ffb7d93b80
> [ 2583.260945] x2 : ffffffffffffffef x1 : ffff80004a3003c0 x0 : ffff07ffb7d93800
> [ 2583.268069] Call trace:
> [ 2583.270504]  __memcpy+0x168/0x260
> [ 2583.273807]  acpi_data_show+0x5c/0x8c
> [ 2583.277464]  sysfs_kf_bin_read+0x78/0xa0
> [ 2583.281378]  kernfs_fop_read_iter+0xac/0x1e0
> [ 2583.285637]  new_sync_read+0xf0/0x184
> [ 2583.289290]  vfs_read+0x158/0x1e4
> [ 2583.292594]  ksys_read+0x74/0x100
> [ 2583.295898]  __arm64_sys_read+0x28/0x34
> [ 2583.299723]  invoke_syscall+0x78/0x100
> [ 2583.303466]  el0_svc_common.constprop.0+0x158/0x160
> [ 2583.308332]  do_el0_svc+0x34/0xa0
> [ 2583.311637]  el0_svc+0x2c/0x54
> [ 2583.314685]  el0_sync_handler+0xa4/0x130
> [ 2583.318596]  el0_sync+0x19c/0x1c0
> [ 2583.321903] Code: a984346c a9c4342c f1010042 54fffee8 (a97c3c8e)
> [ 2583.327989] ---[ end trace e19a85d1dd8510e5 ]---

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

Re: [PATCH v2 6/8] arm64: Import latest memcpy()/memmove() implementation

[dann frazier]

[ + Lorenzo ]
On Sat, May 21, 2022 at 08:56:55AM +0100, Robin Murphy wrote:
> On 2022-05-21 00:30, dann frazier wrote:
> > On Tue, Jun 08, 2021 at 01:42:48PM +0100, Mark Rutland wrote:
> > > On Tue, Jun 08, 2021 at 02:36:26PM +0200, Neil Armstrong wrote:
> > > > On 08/06/2021 14:21, Marek Szyprowski wrote:
> > > > > On 08.06.2021 13:37, Robin Murphy wrote:
> > > > > > On 2021-06-08 12:15, Marek Szyprowski wrote:
> > > > > > > This patch landed recently in linux-next as commit 285133040e6c ("arm64:
> > > > > > > Import latest memcpy()/memmove() implementation"). Sadly it causes
> > > > > > > serious issues on Khadas VIM3 board. Reverting it on top of linux
> > > > > > > next-20210607 (together with 6b8f648959e5 and resolving the conflict in
> > > > > > > the Makefile) fixes the issue. Here is the kernel log:
> > > > > > > 
> > > > > > > Unable to handle kernel paging request at virtual address
> > > > > > > ffff8000136bd204
> > > > > > > Mem abort info:
> > > > > > > � � ESR = 0x96000061
> > > > > > > � � EC = 0x25: DABT (current EL), IL = 32 bits
> > > > > > > � � SET = 0, FnV = 0
> > > > > > > � � EA = 0, S1PTW = 0
> > > > > > > Data abort info:
> > > > > > > � � ISV = 0, ISS = 0x00000061
> > > > > > 
> > > > > > That's an alignment fault, which implies we're accessing something
> > > > > > which isn't normal memory.
> > > 
> > > [...]
> > > 
> > > > > > > I hope that the above log helps fixing the issue. IIRC the SDHCI driver
> > > > > > > on VIM3 board uses internal SRAM for transferring data (instead of DMA),
> > > > > > > so the issue is somehow related to that.
> > > > > > 
> > > > > > Drivers shouldn't be using memcpy() on iomem mappings. Even if they
> > > > > > happen to have got away with it sometimes ;)
> > > > > > 
> > > > > > Taking a quick look at that driver,
> > > > > > 
> > > > > > ����host->bounce_buf = host->regs + SD_EMMC_SRAM_DATA_BUF_OFF;
> > > > > > 
> > > > > > is completely bogus, as Sparse will readily point out.
> > > > 
> > > > My bad, what's the correct way to copy data to an iomem mapping ?
> > > 
> > > We have memcpy_toio() and memcpy_fromio() for this.
> > 
> > ltp's read_all_sys test is triggering something similar which I
> > bisected back to this commit - see below. Does this imply we need
> > something like a memory_read_from_*io*_buffer()?
> 
> Should be fixed soon:
> 
> https://lore.kernel.org/lkml/20220407105120.1280-1-lorenzo.pieralisi@arm.com/

Ah, thanks for the pointer Robin! I'll likely propose this for stable
once it lands (unless there's a reason not to?). My assumption is that
this is a bug unrelated to the new memcpy/memmove implementation, and
that this the new code just made it easier to trip. If that's the
case, I'll plan to propose it for the older stable trees as well.

  -dann

> 
> > [ 2583.023514] Unable to handle kernel paging request at virtual address ffff80004a3003bf
> > [ 2583.031456] Mem abort info:
> > [ 2583.034259]   ESR = 0x96000021
> > [ 2583.037317]   EC = 0x25: DABT (current EL), IL = 32 bits
> > [ 2583.042632]   SET = 0, FnV = 0
> > [ 2583.045689]   EA = 0, S1PTW = 0
> > [ 2583.048834] Data abort info:
> > [ 2583.051704]   ISV = 0, ISS = 0x00000021
> > [ 2583.055542]   CM = 0, WnR = 0
> > [ 2583.058512] swapper pgtable: 4k pages, 48-bit VAs, pgdp=0000401182231000
> > [ 2583.065217] [ffff80004a3003bf] pgd=10000800001a2003, p4d=10000800001a2003, pud=100008000fa35003, pmd=100008001ddbd003, pte=0068000088230f13
> > [ 2583.077751] Internal error: Oops: 96000021 [#22] SMP
> > [ 2583.082710] Modules linked in: nls_iso8859_1 joydev input_leds efi_pstore arm_spe_pmu acpi_ipmi ipmi_ssif arm_cmn xgene_hwmon arm_dmc620_pmu arm_dsu_pmu cppc_cpufreq acpi_tad sch_fq_codel dm_multipath scsi_dh_rdac scsi_dh_emc scsi_dh_alua ipmi_devintf ipmi_msghandler ip_tables x_tables autofs4 btrfs blake2b_generic zstd_compress raid10 raid456 async_raid6_recov async_memcpy async_pq async_xor async_tx xor xor_neon raid6_pq libcrc32c raid1 raid0 multipath linear hid_generic usbhid cdc_ether hid usbnet mlx5_ib ib_uverbs ib_core uas usb_storage ast drm_vram_helper drm_ttm_helper ttm i2c_algo_bit drm_kms_helper syscopyarea crct10dif_ce sysfillrect ghash_ce sysimgblt sha2_ce fb_sys_fops sha256_arm64 cec sha1_ce rc_core mlx5_core nvme drm psample xhci_pci nvme_core mlxfw xhci_pci_renesas tls aes_neon_bs aes_neon_blk aes_ce_blk crypto_simd cryptd aes_ce_cipher
> > [ 2583.158313] CPU: 38 PID: 8392 Comm: read_all Tainted: G      D           5.13.0-rc3+ #15
> > [ 2583.166394] Hardware name: WIWYNN Mt.Jade Server System B81.030Z1.0007/Mt.Jade Motherboard, BIOS 1.6.20210526 (SCP: 1.06.20210526) 2021/05/26
> > [ 2583.179075] pstate: 80400009 (Nzcv daif +PAN -UAO -TCO BTYPE=--)
> > [ 2583.185072] pc : __memcpy+0x168/0x260
> > [ 2583.188735] lr : memory_read_from_buffer+0x58/0x80
> > [ 2583.193524] sp : ffff80004321bb40
> > [ 2583.196826] x29: ffff80004321bb40 x28: ffff07ffdd8bae80 x27: 0000000000000000
> > [ 2583.203952] x26: 0000000000000000 x25: 0000000000000000 x24: ffff07ffd2bd5820
> > [ 2583.211077] x23: ffff80004321bc30 x22: 00000000000003ff x21: ffff80004321bba8
> > [ 2583.218201] x20: 00000000000003ff x19: 00000000000003ff x18: 0000000000000000
> > [ 2583.225326] x17: 0000000000000000 x16: 0000000000000000 x15: 0000000000000000
> > [ 2583.232449] x14: 0000000000000000 x13: 0000000000000000 x12: 0000000000000000
> > [ 2583.239573] x11: 0000000000000000 x10: 0000000000000000 x9 : 0000000000000000
> > [ 2583.246697] x8 : 0000000000000000 x7 : 0000000000000000 x6 : 0000000000000000
> > [ 2583.253820] x5 : ffff07ffb7d93bff x4 : ffff80004a3003ff x3 : ffff07ffb7d93b80
> > [ 2583.260945] x2 : ffffffffffffffef x1 : ffff80004a3003c0 x0 : ffff07ffb7d93800
> > [ 2583.268069] Call trace:
> > [ 2583.270504]  __memcpy+0x168/0x260
> > [ 2583.273807]  acpi_data_show+0x5c/0x8c
> > [ 2583.277464]  sysfs_kf_bin_read+0x78/0xa0
> > [ 2583.281378]  kernfs_fop_read_iter+0xac/0x1e0
> > [ 2583.285637]  new_sync_read+0xf0/0x184
> > [ 2583.289290]  vfs_read+0x158/0x1e4
> > [ 2583.292594]  ksys_read+0x74/0x100
> > [ 2583.295898]  __arm64_sys_read+0x28/0x34
> > [ 2583.299723]  invoke_syscall+0x78/0x100
> > [ 2583.303466]  el0_svc_common.constprop.0+0x158/0x160
> > [ 2583.308332]  do_el0_svc+0x34/0xa0
> > [ 2583.311637]  el0_svc+0x2c/0x54
> > [ 2583.314685]  el0_sync_handler+0xa4/0x130
> > [ 2583.318596]  el0_sync+0x19c/0x1c0
> > [ 2583.321903] Code: a984346c a9c4342c f1010042 54fffee8 (a97c3c8e)
> > [ 2583.327989] ---[ end trace e19a85d1dd8510e5 ]---

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

Re: [PATCH v2 6/8] arm64: Import latest memcpy()/memmove() implementation

[Catalin Marinas]

Hi Peter,

On Thu, Sep 09, 2021 at 05:35:54PM -0700, Peter Collingbourne wrote:
> (apologies for breaking the threading, I wasn't subscribed to
> linux-arm-kernel when you sent this)

lore.kernel.org has some instructions on how to download the message and
reply to it.

> It looks like this patch breaks the KASAN unit tests when running in
> HW tags mode. You can construct a config that reproduces the problem
> with something like:
> 
> make O=out defconfig
> scripts/config --file out/.config -e CONFIG_KUNIT -e CONFIG_KASAN -e
> CONFIG_KASAN_HW_TAGS -e CONFIG_KASAN_KUNIT_TEST
> yes '' | make O=out syncconfig
> 
> With that the "kmalloc_memmove_invalid_size" test fails and causes the
> kernel panic below.
> 
> What appears to be going on is that whereas the old memcpy
> implementation ends up returning early after not copying very much
> data if supplied a "negative" size as a side effect of using
> conditional branches that implement signed comparisons (e.g. b.ge on
> line 75), the new implementation does not exit early and attempts to
> copy a large amount of data backwards 4 bytes (after having disabled
> MTE early on as a result of a tag mismatch) until it hits a read-only
> page and causes a panic.

Treating the size argument as unsigned long is the correct way, it
matches the function prototype.

> I'm not sure what the correct fix should be. It seems that at least
> when KASAN is enabled we should be able to catch these invalid memcpys
> somehow, print an error report and ideally abort the entire operation.
> But we should do so without causing a performance impact in the usual
> case.

I'm not convinced that's a valid test. Do we have any statement
somewhere on what a valid size for memmove/memcpy is? We could
artificially limit it to VA_BITS or even to 63 bits so that it stays a
positive number but the generic memmove() implementation doesn't do such
checks either.

Since the memcpy/memmove routines don't have a fail-safe mode, the only
thing the kernel can do on an MTE fault is to disable tag checking,
report it and continue. What you need instead is copy_*_kernel_nofault
in the kasan tests as they return the number of bytes copied.

My suggestion for a quick fix would be to remove the kasan test.

-- 
Catalin

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

Re: [PATCH v2 6/8] arm64: Import latest memcpy()/memmove() implementation

[Robin Murphy]

On 2021-09-10 01:35, Peter Collingbourne wrote:
> Hi Robin,
> 
> (apologies for breaking the threading, I wasn't subscribed to
> linux-arm-kernel when you sent this)
> 
> It looks like this patch breaks the KASAN unit tests when running in
> HW tags mode. You can construct a config that reproduces the problem
> with something like:
> 
> make O=out defconfig
> scripts/config --file out/.config -e CONFIG_KUNIT -e CONFIG_KASAN -e
> CONFIG_KASAN_HW_TAGS -e CONFIG_KASAN_KUNIT_TEST
> yes '' | make O=out syncconfig
> 
> With that the "kmalloc_memmove_invalid_size" test fails and causes the
> kernel panic below.
> 
> What appears to be going on is that whereas the old memcpy
> implementation ends up returning early after not copying very much
> data if supplied a "negative" size as a side effect of using
> conditional branches that implement signed comparisons (e.g. b.ge on
> line 75), the new implementation does not exit early and attempts to
> copy a large amount of data backwards 4 bytes (after having disabled
> MTE early on as a result of a tag mismatch) until it hits a read-only
> page and causes a panic.

If the test depends on the implementation of memmove() being buggy, it's 
clearly not a very good test :/

> I'm not sure what the correct fix should be. It seems that at least
> when KASAN is enabled we should be able to catch these invalid memcpys
> somehow, print an error report and ideally abort the entire operation.
> But we should do so without causing a performance impact in the usual
> case.

Not sure I follow - the top of the log below clearly shows that KASASN 
*is* detecting and reporting a bad access. The fact that code intended 
to deliberately corrupt memory goes on to corrupt memory doesn't seem 
particularly surprising to me. A bug exists (intentionally or otherwise) 
which led to that invalid access being attempted, and KASAN can't 
magically change that.

Robin.

> 
> Peter
> 
> [    7.726311] ==================================================================
> [    7.726817] BUG: KASAN: invalid-access in __memcpy+0x1a0/0x230
> [    7.727310] Read at addr f8ff000002650ef2 by task kunit_try_catch/117
> [    7.727828] Pointer tag: [f8], memory tag: [fe]
> [    7.728209]
> [    7.728419] CPU: 0 PID: 117 Comm: kunit_try_catch Tainted: G    B
>            5.14.0-rc3-00011-g7a062ce31807-dirty #1
> [    7.729186] Hardware name: linux,dummy-virt (DT)
> [    7.729568] Call trace:
> [    7.729817]  dump_backtrace+0x0/0x1cc
> [    7.730191]  show_stack+0x18/0x24
> [    7.730543]  dump_stack_lvl+0x64/0x7c
> [    7.730917]  print_address_description+0x7c/0x1e0
> [    7.731336]  __kasan_report+0x228/0x30c
> [    7.731718]  kasan_report+0x44/0x70
> [    7.732076]  __do_kernel_fault+0xbc/0x248
> [    7.732464]  do_tag_check_fault+0x38/0xfc
> [    7.732862]  do_mem_abort+0x40/0xb4
> [    7.733223]  el1_abort+0x40/0x60
> [    7.733568]  el1h_64_sync_handler+0x5c/0x98
> [    7.733964]  el1h_64_sync+0x78/0x7c
> [    7.734327]  __memcpy+0x1a0/0x230
> [    7.734684]  kunit_try_run_case+0x48/0x108
> [    7.735073]  kunit_generic_run_threadfn_adapter+0x20/0x2c
> [    7.735536]  kthread+0x170/0x330
> [    7.735888]  ret_from_fork+0x10/0x18
> [    7.736264]
> [    7.736462] Allocated by task 116:
> [    7.736780]  ____kasan_kmalloc+0x108/0x130
> [    7.737163]  __kasan_kmalloc+0x10/0x1c
> [    7.737527]  kthread+0x6c/0x330
> [    7.737861]  ret_from_fork+0x10/0x18
> [    7.738217]
> [    7.738413] Freed by task 115:
> [    7.738713]  kasan_set_track+0x3c/0x74
> [    7.739080]  kasan_set_free_info+0x20/0x2c
> [    7.739461]  ____kasan_slab_free+0x214/0x218
> [    7.739847]  __kasan_slab_free+0x14/0x24
> [    7.740223]  kfree+0x140/0x3bc
> [    7.740553]  free_kthread_struct+0x28/0x4c
> [    7.740935]  __put_task_struct+0x138/0x1b8
> [    7.741326]  delayed_put_task_struct+0x4c/0x88
> [    7.741728]  rcu_do_batch+0x130/0x3ec
> [    7.742092]  rcu_core+0x1f0/0x344
> [    7.742446]  rcu_core_si+0x10/0x1c
> [    7.742796]  __do_softirq+0xd8/0x260
> [    7.743152]
> [    7.743348] The buggy address belongs to the object at ffff000002650e00
> [    7.743348]  which belongs to the cache kmalloc-128 of size 128
> [    7.744152] The buggy address is located 114 bytes to the right of
> [    7.744152]  128-byte region [ffff000002650e00, ffff000002650e80)
> [    7.744971] The buggy address belongs to the page:
> [    7.745345] page:fffffc0000099400 refcount:1 mapcount:0
> mapping:0000000000000000 index:0x0 pfn:0x42650
> [    7.746004] flags:
> 0x3fffc0000000200(slab|node=0|zone=0|lastcpupid=0xffff|kasantag=0x0)
> [    7.746680] raw: 03fffc0000000200 0000000000000000 0000000100000001
> fdff000002401200
> [    7.747276] raw: 0000000000000000 0000000080100010 00000001ffffffff
> 0000000000000000
> [    7.747775] page dumped because: kasan: bad access detected
> [    7.748190]
> [    7.748383] Memory state around the buggy address:
> [    7.748767]  ffff000002650c00: f8 f8 fe fe fe fe fe fe fe fe fe fe
> fe fe fe fe
> [    7.749303]  ffff000002650d00: f8 f8 f8 f8 f8 f8 f8 fe fe fe fe fe
> fe fe fe fe
> [    7.749831] >ffff000002650e00: fa fa fa fa fa fa fa fe fe fe fe fe
> fe fe fe fe
> [    7.750345]                                                                 ^
> [    7.750853]  ffff000002650f00: f8 f8 f8 f8 fe fe fe fe fe fe fe fe
> fe fe fe fe
> [    7.751388]  ffff000002651000: fc fc fc fc fc fc fc fc fe fb fb fb
> fb fb fb fb
> [    7.751914] ==================================================================
> [    7.754424] Unable to handle kernel write to read-only memory at
> virtual address f8ff000002573ff0
> [    7.760818] Mem abort info:
> [    7.761074]   ESR = 0x9600004f
> [    7.761392]   EC = 0x25: DABT (current EL), IL = 32 bits
> [    7.761819]   SET = 0, FnV = 0
> [    7.762086]   EA = 0, S1PTW = 0
> [    7.762374]   FSC = 0x0f: level 3 permission fault
> [    7.763350] Data abort info:
> [    7.763600]   ISV = 0, ISS = 0x0000004f
> [    7.763954]   CM = 0, WnR = 1
> [    7.764282] swapper pgtable: 4k pages, 48-bit VAs, pgdp=000000004193c000
> [    7.764760] [f8ff000002573ff0] pgd=180000005fff7003,
> p4d=180000005fff7003, pud=180000005fff6003, pmd=180000005ffed003,
> pte=0060000042573f87
> [    7.766996] Internal error: Oops: 9600004f [#1] PREEMPT SMP
> [    7.767561] Modules linked in:
> [    7.768138] CPU: 0 PID: 117 Comm: kunit_try_catch Tainted: G    B
>            5.14.0-rc3-00011-g7a062ce31807-dirty #1
> [    7.768948] Hardware name: linux,dummy-virt (DT)
> [    7.769444] pstate: a0400005 (NzCv daif +PAN -UAO -TCO BTYPE=--)
> [    7.769987] pc : __memcpy+0x1e8/0x230
> [    7.771029] lr : kmalloc_memmove_invalid_size+0x148/0x1cc
> [    7.771457] sp : ffff80001023bd60
> [    7.771740] x29: ffff80001023bdb0 x28: 0000000000000000 x27: f8ff000002650d40
> [    7.772450] x26: f3ff000002d64600 x25: ffffc63e374bdf48 x24: ffffc63e3901c50c
> [    7.773109] x23: 000001e700000001 x22: ffffc63e3905f000 x21: ffffc63e38587294
> [    7.773768] x20: ffff80001000bb70 x19: f8ff000002650f00 x18: ffffffffffffffff
> [    7.774422] x17: 0000000000000000 x16: 0000000000000118 x15: 0000000000000004
> [    7.775067] x14: 000000000000000e x13: 42573f83d4202000 x12: d4202000d4202000
> [    7.775722] x11: d4202000d4202000 x10: d4202000d4202000 x9 : d4202000d4202000
> [    7.776397] x8 : d4202000d4202000 x7 : d4202000d4202000 x6 : d4202000d4202000
> [    7.777040] x5 : f8ff000002574030 x4 : f8ff000002573ff4 x3 : 0000000000000000
> [    7.777687] x2 : fffffffffff230b0 x1 : f8ff000002650f04 x0 : f8ff000002650f00
> [    7.778441] Call trace:
> [    7.778714]  __memcpy+0x1e8/0x230
> [    7.779064]  kunit_try_run_case+0x48/0x108
> [    7.779409]  kunit_generic_run_threadfn_adapter+0x20/0x2c
> [    7.779818]  kthread+0x170/0x330
> [    7.780125]  ret_from_fork+0x10/0x18
> [    7.780771] Code: a93e24a8 a97e2488 a93d2caa a97d2c8a (a9bc34ac)
> [    7.781490] ---[ end trace e570c3a10375543e ]---
> [    7.783261] Unable to handle kernel paging request at virtual
> address 00650d50f8fefff8
> [    7.783766] Mem abort info:
> [    7.784002]   ESR = 0x96000004
> [    7.784255]   EC = 0x25: DABT (current EL), IL = 32 bits
> [    7.784637]   SET = 0, FnV = 0
> [    7.784892]   EA = 0, S1PTW = 0
> [    7.785150]   FSC = 0x04: level 0 translation fault
> [    7.785495] Data abort info:
> [    7.785724]   ISV = 0, ISS = 0x00000004
> [    7.786013]   CM = 0, WnR = 0
> [    7.786268] [00650d50f8fefff8] address between user and kernel address ranges
> [    7.786743] Internal error: Oops: 96000004 [#2] PREEMPT SMP
> [    7.787122] Modules linked in:
> [    7.787423] CPU: 0 PID: 117 Comm: kunit_try_catch Tainted: G    B D
>            5.14.0-rc3-00011-g7a062ce31807-dirty #1
> [    7.788099] Hardware name: linux,dummy-virt (DT)
> [    7.788419] pstate: 004000c5 (nzcv daIF +PAN -UAO -TCO BTYPE=--)
> [    7.788855] pc : swake_up_locked+0x1c/0x48
> [    7.789197] lr : complete+0x44/0x64
> [    7.789508] sp : ffff80001023b910
> [    7.789773] x29: ffff80001023b910 x28: f3ff000002d64600 x27: f3ff000002d64600
> [    7.790425] x26: f3ff000002d64600 x25: ffffc63e374bdf48 x24: ffffc63e374efbf8
> [    7.791069] x23: ffff80001023baa4 x22: 0000000000000000 x21: 0000000000000000
> [    7.791733] x20: f8ff000002650d40 x19: 02650d50f8ff0000 x18: ffffffffffffffff
> [    7.792380] x17: 0000000000000000 x16: 0000000000000118 x15: 0000000000000004
> [    7.793031] x14: 0000000000000fff x13: ffffc63e38d4aa08 x12: 0000000000000003
> [    7.793683] x11: 0000000000000000 x10: 0000000000000002 x9 : 0000000000000000
> [    7.794341] x8 : 0000000000000001 x7 : 205d303934313837 x6 : 372e37202020205b
> [    7.794977] x5 : ffffc63e39041f58 x4 : 0000000000000001 x3 : 0000000000000000
> [    7.795622] x2 : 0000000000000001 x1 : 0000000000000000 x0 : f8ff000002650d50
> [    7.796272] Call trace:
> [    7.796503]  swake_up_locked+0x1c/0x48
> [    7.796836]  complete+0x44/0x64
> [    7.797133]  mm_release+0xd4/0xec
> [    7.797437]  exit_mm_release+0x28/0x38
> [    7.797775]  exit_mm+0x3c/0x214
> [    7.798074]  do_exit+0x180/0x778
> [    7.798379]  die+0x2ac/0x2b0
> [    7.798669]  __do_kernel_fault+0x1bc/0x248
> [    7.799020]  do_page_fault+0x4c/0x3c4
> [    7.799343]  do_mem_abort+0x40/0xb4
> [    7.799661]  el1_abort+0x40/0x60
> [    7.799968]  el1h_64_sync_handler+0x5c/0x98
> [    7.800318]  el1h_64_sync+0x78/0x7c
> [    7.800639]  __memcpy+0x1e8/0x230
> [    7.800948]  kunit_try_run_case+0x48/0x108
> [    7.801297]  kunit_generic_run_threadfn_adapter+0x20/0x2c
> [    7.801697]  kthread+0x170/0x330
> [    7.801993]  ret_from_fork+0x10/0x18
> [    7.802383] Code: 910003fd f8408c13 eb00027f 54000100 (f85f8260)
> [    7.802810] ---[ end trace e570c3a10375543f ]---
> [    7.803154] note: kunit_try_catch[117] exited with preempt_count 2
> [    7.803596] Fixing recursive fault but reboot is needed!
> [   10.659826] Unable to handle kernel paging request at virtual
> address ffff39c2e762363e
> [   10.661382] Mem abort info:
> [   10.662059]   ESR = 0x96000004
> [   10.662795]   EC = 0x25: DABT (current EL), IL = 32 bits
> [   10.663887]   SET = 0, FnV = 0
> [   10.664634]   EA = 0, S1PTW = 0
> [   10.665390]   FSC = 0x04: level 0 translation fault
> [   10.666379] Data abort info:
> [   10.667043]   ISV = 0, ISS = 0x00000004
> [   10.667879]   CM = 0, WnR = 0
> [   10.668605] swapper pgtable: 4k pages, 48-bit VAs, pgdp=000000004193c000
> [   10.669507] [ffff39c2e762363e] pgd=0000000000000000, p4d=0000000000000000
> [   10.670636] Internal error: Oops: 96000004 [#3] PREEMPT SMP
> [   10.671425] Modules linked in:
> [   10.671998] CPU: 0 PID: 0 Comm: swapper/0 Tainted: G    B D
>    5.14.0-rc3-00011-g7a062ce31807-dirty #1
> [   10.673281] Hardware name: linux,dummy-virt (DT)
> [   10.673958] pstate: 604000c5 (nZCv daIF +PAN -UAO -TCO BTYPE=--)
> [   10.674844] pc : __queue_work+0x1f4/0x408
> [   10.675536] lr : __queue_work+0x38/0x408
> [   10.676205] sp : ffff800010003d80
> [   10.676739] x29: ffff800010003d80 x28: ffff39c2e762363e x27: dead000000000122
> [   10.678061] x26: 0000000000000000 x25: ffffc63e38d1d768 x24: 000000007fffffff
> [   10.679392] x23: ffffc63e388a9638 x22: 0000000000000014 x21: 0000000000000100
> [   10.680286] x20: f9ff0000025f8c00 x19: f8ff000002fcc5b0 x18: 0000000000000000
> [   10.681183] x17: 0000000000000000 x16: 0000000000000001 x15: 0000000000000000
> [   10.682071] x14: 0000000000200000 x13: f8ff000002fcc588 x12: ffff00001fecd258
> [   10.682959] x11: ffffffffc0000040 x10: ffffc63e3889f000 x9 : ffff39c1e7627000
> [   10.683867] x8 : 000000007fffffff x7 : 00001e8480000000 x6 : 0000000000000000
> [   10.684763] x5 : 0000000000000000 x4 : 0000000000000000 x3 : 0000000000000069
> [   10.685642] x2 : f8ff000002fcc5b0 x1 : f9ff0000025f8c00 x0 : ffffc63e3901c250
> [   10.686511] Call trace:
> [   10.686832]  __queue_work+0x1f4/0x408
> [   10.687287]  queue_work_on+0x54/0x8c
> [   10.687736]  blk_rq_timed_out_timer+0x20/0x2c
> [   10.688225]  __run_timers+0x1d8/0x2fc
> [   10.688682]  run_timer_softirq+0x24/0x48
> [   10.689154]  __do_softirq+0xd8/0x260
> [   10.689592]  __irq_exit_rcu+0xf4/0xf8
> [   10.689954]  irq_exit+0x10/0x20
> [   10.690265]  handle_domain_irq+0x60/0x88
> [   10.690609]  gic_handle_irq+0x68/0xe0
> [   10.690944]  call_on_irq_stack+0x2c/0x54
> [   10.691284]  el1_interrupt+0x60/0xa4
> [   10.691633]  el1h_64_irq_handler+0x18/0x24
> [   10.691994]  el1h_64_irq+0x78/0x7c
> [   10.692316]  arch_cpu_idle+0x18/0x28
> [   10.692651]  cpuidle_idle_call+0x70/0x250
> [   10.693011]  do_idle+0xb0/0xfc
> [   10.693317]  cpu_startup_entry+0x24/0x28
> [   10.693662]  rest_init+0xe4/0xf4
> [   10.693976]  arch_call_rest_init+0x10/0x1c
> [   10.694343]  start_kernel+0x328/0x43c
> [   10.694679]  __primary_switched+0xc0/0xc8
> [   10.695101] Code: eb14013f 54000280 aa1b03e0 9439f6fb (f9400380)
> [   10.695897] ---[ end trace e570c3a103755440 ]---
> [   10.696360] Kernel panic - not syncing: Oops: Fatal exception in interrupt
> [   10.697132] Kernel Offset: 0x463e27000000 from 0xffff800010000000
> [   10.697565] PHYS_OFFSET: 0x40000000
> [   10.697857] CPU features: 0x000003d9,4d330c5f
> [   10.698416] Memory Limit: none
> [   10.698918] ---[ end Kernel panic - not syncing: Oops: Fatal
> exception in interrupt ]---
> 

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

Re: [PATCH v2 6/8] arm64: Import latest memcpy()/memmove() implementation

[Peter Collingbourne]

On Fri, Sep 10, 2021 at 4:42 AM Robin Murphy <robin.murphy@arm.com> wrote:
>
> On 2021-09-10 01:35, Peter Collingbourne wrote:
> > Hi Robin,
> >
> > (apologies for breaking the threading, I wasn't subscribed to
> > linux-arm-kernel when you sent this)
> >
> > It looks like this patch breaks the KASAN unit tests when running in
> > HW tags mode. You can construct a config that reproduces the problem
> > with something like:
> >
> > make O=out defconfig
> > scripts/config --file out/.config -e CONFIG_KUNIT -e CONFIG_KASAN -e
> > CONFIG_KASAN_HW_TAGS -e CONFIG_KASAN_KUNIT_TEST
> > yes '' | make O=out syncconfig
> >
> > With that the "kmalloc_memmove_invalid_size" test fails and causes the
> > kernel panic below.
> >
> > What appears to be going on is that whereas the old memcpy
> > implementation ends up returning early after not copying very much
> > data if supplied a "negative" size as a side effect of using
> > conditional branches that implement signed comparisons (e.g. b.ge on
> > line 75), the new implementation does not exit early and attempts to
> > copy a large amount of data backwards 4 bytes (after having disabled
> > MTE early on as a result of a tag mismatch) until it hits a read-only
> > page and causes a panic.
>
> If the test depends on the implementation of memmove() being buggy, it's
> clearly not a very good test :/
>
> > I'm not sure what the correct fix should be. It seems that at least
> > when KASAN is enabled we should be able to catch these invalid memcpys
> > somehow, print an error report and ideally abort the entire operation.
> > But we should do so without causing a performance impact in the usual
> > case.
>
> Not sure I follow - the top of the log below clearly shows that KASASN
> *is* detecting and reporting a bad access. The fact that code intended
> to deliberately corrupt memory goes on to corrupt memory doesn't seem
> particularly surprising to me. A bug exists (intentionally or otherwise)
> which led to that invalid access being attempted, and KASAN can't
> magically change that.

That makes sense. So we want to make sure that:

a) memmove overflows trigger an error report
b) we can safely continue execution

It seems the best way to ensure both of these properties is to have
the memmove do an out-of-bounds read -- all writes would then be into
the correct allocation. That way, we should see an error report but no
memory corruption. We can do that by changing the memmove call to only
copy "size" bytes.

When I was about to send my patch I noticed that Andrey already sent a
fix for this in commit 1b0668be62cf that disables the memmove test
when KASAN HW tags is enabled. I reckon there's still some value in
testing a) in HW tags mode, so I went ahead and had my patch also
revert his.

Peter

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

[PATCH v2 7/8] arm64: Better optimised memchr()

[Robin Murphy]

Although we implement our own assembly version of memchr(), it turns
out to be barely any better than what GCC can generate for the generic
C version (and would go wrong if the size_t argument were ever large
enough to be interpreted as negative). Unfortunately we can't import the
tuned implementation from the Arm optimized-routines library, since that
has some Advanced SIMD parts which are not really viable for general
kernel library code. What we can do, however, is pep things up with some
relatively straightforward word-at-a-time logic for larger calls.

Adding some timing to optimized-routines' memchr() test for a simple
benchmark, overall this version comes in around half as fast as the SIMD
code, but still nearly 4x faster than our existing implementation.

Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
 arch/arm64/lib/memchr.S | 65 +++++++++++++++++++++++++++++++++--------
 1 file changed, 53 insertions(+), 12 deletions(-)

diff --git a/arch/arm64/lib/memchr.S b/arch/arm64/lib/memchr.S
index edf6b970a277..7c2276fdab54 100644
--- a/arch/arm64/lib/memchr.S
+++ b/arch/arm64/lib/memchr.S
@@ -1,9 +1,6 @@
 /* SPDX-License-Identifier: GPL-2.0-only */
 /*
- * Based on arch/arm/lib/memchr.S
- *
- * Copyright (C) 1995-2000 Russell King
- * Copyright (C) 2013 ARM Ltd.
+ * Copyright (C) 2021 Arm Ltd.
  */
 
 #include <linux/linkage.h>
@@ -19,16 +16,60 @@
  * Returns:
  *	x0 - address of first occurrence of 'c' or 0
  */
+
+#define L(label) .L ## label
+
+#define REP8_01 0x0101010101010101
+#define REP8_7f 0x7f7f7f7f7f7f7f7f
+
+#define srcin		x0
+#define chrin		w1
+#define cntin		x2
+
+#define result		x0
+
+#define wordcnt		x3
+#define rep01		x4
+#define repchr		x5
+#define cur_word	x6
+#define cur_byte	w6
+#define tmp		x7
+#define tmp2		x8
+
+	.p2align 4
+	nop
 SYM_FUNC_START_WEAK_PI(memchr)
-	and	w1, w1, #0xff
-1:	subs	x2, x2, #1
-	b.mi	2f
-	ldrb	w3, [x0], #1
-	cmp	w3, w1
-	b.ne	1b
-	sub	x0, x0, #1
+	and	chrin, chrin, #0xff
+	lsr	wordcnt, cntin, #3
+	cbz	wordcnt, L(byte_loop)
+	mov	rep01, #REP8_01
+	mul	repchr, x1, rep01
+	and	cntin, cntin, #7
+L(word_loop):
+	ldr	cur_word, [srcin], #8
+	sub	wordcnt, wordcnt, #1
+	eor	cur_word, cur_word, repchr
+	sub	tmp, cur_word, rep01
+	orr	tmp2, cur_word, #REP8_7f
+	bics	tmp, tmp, tmp2
+	b.ne	L(found_word)
+	cbnz	wordcnt, L(word_loop)
+L(byte_loop):
+	cbz	cntin, L(not_found)
+	ldrb	cur_byte, [srcin], #1
+	sub	cntin, cntin, #1
+	cmp	cur_byte, chrin
+	b.ne	L(byte_loop)
+	sub	srcin, srcin, #1
 	ret
-2:	mov	x0, #0
+L(found_word):
+CPU_LE(	rev	tmp, tmp)
+	clz	tmp, tmp
+	sub	tmp, tmp, #64
+	add	result, srcin, tmp, asr #3
+	ret
+L(not_found):
+	mov	result, #0
 	ret
 SYM_FUNC_END_PI(memchr)
 EXPORT_SYMBOL_NOKASAN(memchr)
-- 
2.21.0.dirty


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

[PATCH v2 8/8] arm64: Rewrite __arch_clear_user()

[Robin Murphy]

Now that we're always using STTR variants rather than abstracting two
different addressing modes, the user_ldst macro here is frankly more
obfuscating than helpful. Rewrite __arch_clear_user() with regular
USER() annotations so that it's clearer what's going on, and take the
opportunity to minimise the branchiness in the most common paths, while
also allowing the exception fixup to return an accurate result.

Apparently some folks examine large reads from /dev/zero closely enough
to notice the loop being hot, so align it per the other critical loops
(presumably around a typical instruction fetch granularity).

Reviewed-by: Mark Rutland <mark.rutland@arm.com>
Signed-off-by: Robin Murphy <robin.murphy@arm.com>
---
 arch/arm64/lib/clear_user.S | 47 +++++++++++++++++++++----------------
 1 file changed, 27 insertions(+), 20 deletions(-)

diff --git a/arch/arm64/lib/clear_user.S b/arch/arm64/lib/clear_user.S
index af9afcbec92c..a7efb2ad2a1c 100644
--- a/arch/arm64/lib/clear_user.S
+++ b/arch/arm64/lib/clear_user.S
@@ -1,12 +1,9 @@
 /* SPDX-License-Identifier: GPL-2.0-only */
 /*
- * Based on arch/arm/lib/clear_user.S
- *
- * Copyright (C) 2012 ARM Ltd.
+ * Copyright (C) 2021 Arm Ltd.
  */
-#include <linux/linkage.h>
 
-#include <asm/asm-uaccess.h>
+#include <linux/linkage.h>
 #include <asm/assembler.h>
 
 	.text
@@ -19,25 +16,33 @@
  *
  * Alignment fixed up by hardware.
  */
+
+	.p2align 4
+	// Alignment is for the loop, but since the prologue (including BTI)
+	// is also 16 bytes we can keep any padding outside the function
 SYM_FUNC_START(__arch_clear_user)
-	mov	x2, x1			// save the size for fixup return
+	add	x2, x0, x1
 	subs	x1, x1, #8
 	b.mi	2f
 1:
-user_ldst 9f, sttr, xzr, x0, 8
+USER(9f, sttr	xzr, [x0])
+	add	x0, x0, #8
 	subs	x1, x1, #8
-	b.pl	1b
-2:	adds	x1, x1, #4
-	b.mi	3f
-user_ldst 9f, sttr, wzr, x0, 4
-	sub	x1, x1, #4
-3:	adds	x1, x1, #2
-	b.mi	4f
-user_ldst 9f, sttrh, wzr, x0, 2
-	sub	x1, x1, #2
-4:	adds	x1, x1, #1
-	b.mi	5f
-user_ldst 9f, sttrb, wzr, x0, 0
+	b.hi	1b
+USER(9f, sttr	xzr, [x2, #-8])
+	mov	x0, #0
+	ret
+
+2:	tbz	x1, #2, 3f
+USER(9f, sttr	wzr, [x0])
+USER(8f, sttr	wzr, [x2, #-4])
+	mov	x0, #0
+	ret
+
+3:	tbz	x1, #1, 4f
+USER(9f, sttrh	wzr, [x0])
+4:	tbz	x1, #0, 5f
+USER(7f, sttrb	wzr, [x2, #-1])
 5:	mov	x0, #0
 	ret
 SYM_FUNC_END(__arch_clear_user)
@@ -45,6 +50,8 @@ EXPORT_SYMBOL(__arch_clear_user)
 
 	.section .fixup,"ax"
 	.align	2
-9:	mov	x0, x2			// return the original size
+7:	sub	x0, x2, #5	// Adjust for faulting on the final byte...
+8:	add	x0, x0, #4	// ...or the second word of the 4-7 byte case
+9:	sub	x0, x2, x0
 	ret
 	.previous
-- 
2.21.0.dirty


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

Re: [PATCH v2 0/8] arm64: String function updates

[Will Deacon]

On Thu, 27 May 2021 16:34:40 +0100, Robin Murphy wrote:
> Here's a proper update with the relicensing clarified and Mark's other
> review comments addressed on patch #8.
> 
> v1: https://lore.kernel.org/linux-arm-kernel/cover.1620738177.git.robin.murphy@arm.com/
> 
> Thanks,
> Robin.
> 
> [...]

Applied to arm64 (for-next/cortex-strings), thanks!

Note that I haven't yet put this into -next, as I'd like to have a clean
CI run back with these applied first.

[1/8] arm64: Import latest version of Cortex Strings' memcmp
      https://git.kernel.org/arm64/c/43de30d36742
[2/8] arm64: Import latest version of Cortex Strings' strcmp
      https://git.kernel.org/arm64/c/758602c04409
[3/8] arm64: Import updated version of Cortex Strings' strlen
      https://git.kernel.org/arm64/c/325a1de81287
[4/8] arm64: Import latest version of Cortex Strings' strncmp
      https://git.kernel.org/arm64/c/020b199bc70d
[5/8] arm64: Add assembly annotations for weak-PI-alias madness
      https://git.kernel.org/arm64/c/b6c4ea48415d
[6/8] arm64: Import latest memcpy()/memmove() implementation
      https://git.kernel.org/arm64/c/285133040e6c
[7/8] arm64: Better optimised memchr()
      https://git.kernel.org/arm64/c/9e51cafd783b
[8/8] arm64: Rewrite __arch_clear_user()
      https://git.kernel.org/arm64/c/344323e0428b

Cheers,
-- 
Will

https://fixes.arm64.dev
https://next.arm64.dev
https://will.arm64.dev

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