Re: [PATCH] mtd: spear_smi: Fix nonalignment not handled in memcpy_toio

[Miquel Raynal <miquel.raynal@bootlin.com>]

[-- Attachment #1: Type: text/plain, Size: 4291 bytes --]


Russel was out of the loop, re-adding him as he may have interesting
thoughts about this. Sorry for the double e-mail.

Miquel Raynal <miquel.raynal@bootlin.com> wrote on Tue, 22 Oct 2019
09:44:51 +0200:

> Hello,
> 
> Boris Brezillon <boris.brezillon@collabora.com> wrote on Mon, 21 Oct
> 2019 10:01:05 +0200:
> 
> > On Fri, 18 Oct 2019 16:36:43 +0200
> > Miquel Raynal <miquel.raynal@bootlin.com> wrote:
> >   
> > > Any write with either dd or flashcp to a device driven by the
> > > spear_smi.c driver will pass through the spear_smi_cpy_toio()
> > > function. This function will get called for chunks of up to 256 bytes.
> > > If the amount of data is smaller, we may have a problem if the data
> > > length is not 4-byte aligned. In this situation, the kernel panics
> > > during the memcpy:
> > > 
> > >     # dd if=/dev/urandom bs=1001 count=1 of=/dev/mtd6
> > >     spear_smi_cpy_toio [620] dest c9070000, src c7be8800, len 256
> > >     spear_smi_cpy_toio [620] dest c9070100, src c7be8900, len 256
> > >     spear_smi_cpy_toio [620] dest c9070200, src c7be8a00, len 256
> > >     spear_smi_cpy_toio [620] dest c9070300, src c7be8b00, len 233
> > >     Unhandled fault: external abort on non-linefetch (0x808) at 0xc90703e8
> > >     [...]
> > >     PC is at memcpy+0xcc/0x330    
> > 
> > Can you find out which instruction is at memcpy+0xcc/0x330? For the
> > record, the assembly is here [1].  
> 
> The full disassembled file is attached, here is the failing part:
> 
> 7:			ldmfd	sp!, {r5 - r8}
>   b8:	e8bd01e0 	pop	{r5, r6, r7, r8}
> 	UNWIND(		.fnend				) @ end of second stmfd block
> 
> 	UNWIND(		.fnstart			)
> 			usave	r4, lr			  @ still in first stmdb block
> 8:			movs	r2, r2, lsl #31
>   bc:	e1b02f82 	lsls	r2, r2, #31
> 			ldr1b	r1, r3, ne, abort=21f
>   c0:	14d13001 	ldrbne	r3, [r1], #1
> 			ldr1b	r1, r4, cs, abort=21f
>   c4:	24d14001 	ldrbcs	r4, [r1], #1
> 			ldr1b	r1, ip, cs, abort=21f
>   c8:	24d1c001 	ldrbcs	ip, [r1], #1
> 			str1b	r0, r3, ne, abort=21f
>   cc:	14c03001 	strbne	r3, [r0], #1
> 			str1b	r0, r4, cs, abort=21f
>   d0:	24c04001 	strbcs	r4, [r0], #1
> 			str1b	r0, ip, cs, abort=21f
>   d4:	24c0c001 	strbcs	ip, [r0], #1
> 
> 			exit	r4, pc
>   d8:	e8bd8011 	pop	{r0, r4, pc}
> 
> 
> So the fault is triggered on a strbne instruction.
> 
> > > 
> > > Workaround this issue by using the alternate _memcpy_toio() method
> > > which at least does not present the same problem.
> > > 
> > > Fixes: f18dbbb1bfe0 ("mtd: ST SPEAr: Add SMI driver for serial NOR flash")
> > > Cc: stable@vger.kernel.org
> > > Suggested-by: Boris Brezillon <boris.brezillon@collabora.com>    
> > 
> > I don't remember suggesting that as a final solution. I probably
> > suggested to test with _memcpy_toio() to see if using a byte accessor
> > was fixing the problem, but it's definitely not the right solution
> > (using byte access with a memory barrier for 256 bytes buffers is likely
> > to cause a huge perf penalty).
> >   
> > > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > > ---
> > > 
> > > Hello,
> > > 
> > > This patch could not be tested with a mainline kernel (only compiled)
> > > but was tested with a stable 4.14.x kernel. I have really no idea why
> > > memcpy fails in this situation that's why I propose this workaround
> > > but I bet there is something deeper not working.
> > > 
> > > Thanks,
> > > Miquèl
> > > 
> > >  drivers/mtd/devices/spear_smi.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/mtd/devices/spear_smi.c b/drivers/mtd/devices/spear_smi.c
> > > index 986f81d2f93e..d888625a3244 100644
> > > --- a/drivers/mtd/devices/spear_smi.c
> > > +++ b/drivers/mtd/devices/spear_smi.c
> > > @@ -614,7 +614,7 @@ static inline int spear_smi_cpy_toio(struct spear_smi *dev, u32 bank,
> > >  	ctrlreg1 = readl(dev->io_base + SMI_CR1);
> > >  	writel((ctrlreg1 | WB_MODE) & ~SW_MODE, dev->io_base + SMI_CR1);
> > >  
> > > -	memcpy_toio(dest, src, len);
> > > +	_memcpy_toio(dest, src, len);
> > >  
> > >  	writel(ctrlreg1, dev->io_base + SMI_CR1);
> > >      
> > 
> > [1]https://elixir.bootlin.com/linux/v5.4-rc2/source/arch/arm/lib/memcpy.S  
> 

Thanks,
Miquèl

[-- Attachment #2: memcpy-disassemble --]
[-- Type: application/octet-stream, Size: 9987 bytes --]


arch/arm/lib/memcpy.o:     file format elf32-littlearm


Disassembly of section .text:

00000000 <memcpy>:
 *	than one 32bit instruction in Thumb-2)
 */


	UNWIND(	.fnstart			)
		enter	r4, lr
   0:	e92d4011 	push	{r0, r4, lr}
	UNWIND(	.fnend				)

	UNWIND(	.fnstart			)
		usave	r4, lr			  @ in first stmdb block

		subs	r2, r2, #4
   4:	e2522004 	subs	r2, r2, #4
		blt	8f
   8:	ba00002b 	blt	bc <memcpy+0xbc>
		ands	ip, r0, #3
   c:	e210c003 	ands	ip, r0, #3
	PLD(	pld	[r1, #0]		)
  10:	f5d1f000 	pld	[r1]
		bne	9f
  14:	1a000030 	bne	dc <memcpy+0xdc>
		ands	ip, r1, #3
  18:	e211c003 	ands	ip, r1, #3
		bne	10f
  1c:	1a00003a 	bne	10c <memcpy+0x10c>

1:		subs	r2, r2, #(28)
  20:	e252201c 	subs	r2, r2, #28
		stmfd	sp!, {r5 - r8}
  24:	e92d01e0 	push	{r5, r6, r7, r8}
	UNWIND(	.fnend				)

	UNWIND(	.fnstart			)
		usave	r4, lr
	UNWIND(	.save	{r5 - r8}		) @ in second stmfd block
		blt	5f
  28:	ba00000c 	blt	60 <memcpy+0x60>
	CALGN(	bcs	2f			)
	CALGN(	adr	r4, 6f			)
	CALGN(	subs	r2, r2, r3		)  @ C gets set
	CALGN(	add	pc, r4, ip		)

	PLD(	pld	[r1, #0]		)
  2c:	f5d1f000 	pld	[r1]
2:	PLD(	subs	r2, r2, #96		)
  30:	e2522060 	subs	r2, r2, #96	; 0x60
	PLD(	pld	[r1, #28]		)
  34:	f5d1f01c 	pld	[r1, #28]
	PLD(	blt	4f			)
  38:	ba000002 	blt	48 <memcpy+0x48>
	PLD(	pld	[r1, #60]		)
  3c:	f5d1f03c 	pld	[r1, #60]	; 0x3c
	PLD(	pld	[r1, #92]		)
  40:	f5d1f05c 	pld	[r1, #92]	; 0x5c

3:	PLD(	pld	[r1, #124]		)
  44:	f5d1f07c 	pld	[r1, #124]	; 0x7c
4:		ldr8w	r1, r3, r4, r5, r6, r7, r8, ip, lr, abort=20f
  48:	e8b151f8 	ldm	r1!, {r3, r4, r5, r6, r7, r8, ip, lr}
		subs	r2, r2, #32
  4c:	e2522020 	subs	r2, r2, #32
		str8w	r0, r3, r4, r5, r6, r7, r8, ip, lr, abort=20f
  50:	e8a051f8 	stmia	r0!, {r3, r4, r5, r6, r7, r8, ip, lr}
		bge	3b
  54:	aafffffa 	bge	44 <memcpy+0x44>
	PLD(	cmn	r2, #96			)
  58:	e3720060 	cmn	r2, #96	; 0x60
	PLD(	bge	4b			)
  5c:	aafffff9 	bge	48 <memcpy+0x48>

5:		ands	ip, r2, #28
  60:	e212c01c 	ands	ip, r2, #28
		rsb	ip, ip, #32
  64:	e26cc020 	rsb	ip, ip, #32
#if LDR1W_SHIFT > 0
		lsl	ip, ip, #LDR1W_SHIFT
#endif
		addne	pc, pc, ip		@ C is always clear here
  68:	108ff00c 	addne	pc, pc, ip
		b	7f
  6c:	ea000011 	b	b8 <memcpy+0xb8>
6:
		.rept	(1 << LDR1W_SHIFT)
		W(nop)
		.endr
  70:	e1a00000 	nop			; (mov r0, r0)
		ldr1w	r1, r3, abort=20f
  74:	e4913004 	ldr	r3, [r1], #4
		ldr1w	r1, r4, abort=20f
  78:	e4914004 	ldr	r4, [r1], #4
		ldr1w	r1, r5, abort=20f
  7c:	e4915004 	ldr	r5, [r1], #4
		ldr1w	r1, r6, abort=20f
  80:	e4916004 	ldr	r6, [r1], #4
		ldr1w	r1, r7, abort=20f
  84:	e4917004 	ldr	r7, [r1], #4
		ldr1w	r1, r8, abort=20f
  88:	e4918004 	ldr	r8, [r1], #4
		ldr1w	r1, lr, abort=20f
  8c:	e491e004 	ldr	lr, [r1], #4
#if LDR1W_SHIFT < STR1W_SHIFT
		lsl	ip, ip, #STR1W_SHIFT - LDR1W_SHIFT
#elif LDR1W_SHIFT > STR1W_SHIFT
		lsr	ip, ip, #LDR1W_SHIFT - STR1W_SHIFT
#endif
		add	pc, pc, ip
  90:	e08ff00c 	add	pc, pc, ip
		nop
  94:	e1a00000 	nop			; (mov r0, r0)
		.rept	(1 << STR1W_SHIFT)
		W(nop)
		.endr
  98:	e1a00000 	nop			; (mov r0, r0)
		str1w	r0, r3, abort=20f
  9c:	e4803004 	str	r3, [r0], #4
		str1w	r0, r4, abort=20f
  a0:	e4804004 	str	r4, [r0], #4
		str1w	r0, r5, abort=20f
  a4:	e4805004 	str	r5, [r0], #4
		str1w	r0, r6, abort=20f
  a8:	e4806004 	str	r6, [r0], #4
		str1w	r0, r7, abort=20f
  ac:	e4807004 	str	r7, [r0], #4
		str1w	r0, r8, abort=20f
  b0:	e4808004 	str	r8, [r0], #4
		str1w	r0, lr, abort=20f
  b4:	e480e004 	str	lr, [r0], #4

	CALGN(	bcs	2b			)

7:		ldmfd	sp!, {r5 - r8}
  b8:	e8bd01e0 	pop	{r5, r6, r7, r8}
	UNWIND(	.fnend				) @ end of second stmfd block

	UNWIND(	.fnstart			)
		usave	r4, lr			  @ still in first stmdb block
8:		movs	r2, r2, lsl #31
  bc:	e1b02f82 	lsls	r2, r2, #31
		ldr1b	r1, r3, ne, abort=21f
  c0:	14d13001 	ldrbne	r3, [r1], #1
		ldr1b	r1, r4, cs, abort=21f
  c4:	24d14001 	ldrbcs	r4, [r1], #1
		ldr1b	r1, ip, cs, abort=21f
  c8:	24d1c001 	ldrbcs	ip, [r1], #1
		str1b	r0, r3, ne, abort=21f
  cc:	14c03001 	strbne	r3, [r0], #1
		str1b	r0, r4, cs, abort=21f
  d0:	24c04001 	strbcs	r4, [r0], #1
		str1b	r0, ip, cs, abort=21f
  d4:	24c0c001 	strbcs	ip, [r0], #1

		exit	r4, pc
  d8:	e8bd8011 	pop	{r0, r4, pc}

9:		rsb	ip, ip, #4
  dc:	e26cc004 	rsb	ip, ip, #4
		cmp	ip, #2
  e0:	e35c0002 	cmp	ip, #2
		ldr1b	r1, r3, gt, abort=21f
  e4:	c4d13001 	ldrbgt	r3, [r1], #1
		ldr1b	r1, r4, ge, abort=21f
  e8:	a4d14001 	ldrbge	r4, [r1], #1
		ldr1b	r1, lr, abort=21f
  ec:	e4d1e001 	ldrb	lr, [r1], #1
		str1b	r0, r3, gt, abort=21f
  f0:	c4c03001 	strbgt	r3, [r0], #1
		str1b	r0, r4, ge, abort=21f
  f4:	a4c04001 	strbge	r4, [r0], #1
		subs	r2, r2, ip
  f8:	e052200c 	subs	r2, r2, ip
		str1b	r0, lr, abort=21f
  fc:	e4c0e001 	strb	lr, [r0], #1
		blt	8b
 100:	baffffed 	blt	bc <memcpy+0xbc>
		ands	ip, r1, #3
 104:	e211c003 	ands	ip, r1, #3
		beq	1b
 108:	0affffc4 	beq	20 <memcpy+0x20>

10:		bic	r1, r1, #3
 10c:	e3c11003 	bic	r1, r1, #3
		cmp	ip, #2
 110:	e35c0002 	cmp	ip, #2
		ldr1w	r1, lr, abort=21f
 114:	e491e004 	ldr	lr, [r1], #4
		beq	17f
 118:	0a00002c 	beq	1d0 <memcpy+0x1d0>
		bgt	18f
 11c:	ca000057 	bgt	280 <memcpy+0x280>
	UNWIND(	.fnend				)

		.endm


		forward_copy_shift	pull=8	push=24
 120:	e252201c 	subs	r2, r2, #28
 124:	ba00001f 	blt	1a8 <memcpy+0x1a8>
 128:	e92d03e0 	push	{r5, r6, r7, r8, r9}
 12c:	f5d1f000 	pld	[r1]
 130:	e2522060 	subs	r2, r2, #96	; 0x60
 134:	f5d1f01c 	pld	[r1, #28]
 138:	ba000002 	blt	148 <memcpy+0x148>
 13c:	f5d1f03c 	pld	[r1, #60]	; 0x3c
 140:	f5d1f05c 	pld	[r1, #92]	; 0x5c
 144:	f5d1f07c 	pld	[r1, #124]	; 0x7c
 148:	e8b100f0 	ldm	r1!, {r4, r5, r6, r7}
 14c:	e1a0342e 	lsr	r3, lr, #8
 150:	e2522020 	subs	r2, r2, #32
 154:	e8b15300 	ldm	r1!, {r8, r9, ip, lr}
 158:	e1833c04 	orr	r3, r3, r4, lsl #24
 15c:	e1a04424 	lsr	r4, r4, #8
 160:	e1844c05 	orr	r4, r4, r5, lsl #24
 164:	e1a05425 	lsr	r5, r5, #8
 168:	e1855c06 	orr	r5, r5, r6, lsl #24
 16c:	e1a06426 	lsr	r6, r6, #8
 170:	e1866c07 	orr	r6, r6, r7, lsl #24
 174:	e1a07427 	lsr	r7, r7, #8
 178:	e1877c08 	orr	r7, r7, r8, lsl #24
 17c:	e1a08428 	lsr	r8, r8, #8
 180:	e1888c09 	orr	r8, r8, r9, lsl #24
 184:	e1a09429 	lsr	r9, r9, #8
 188:	e1899c0c 	orr	r9, r9, ip, lsl #24
 18c:	e1a0c42c 	lsr	ip, ip, #8
 190:	e18ccc0e 	orr	ip, ip, lr, lsl #24
 194:	e8a013f8 	stmia	r0!, {r3, r4, r5, r6, r7, r8, r9, ip}
 198:	aaffffe9 	bge	144 <memcpy+0x144>
 19c:	e3720060 	cmn	r2, #96	; 0x60
 1a0:	aaffffe8 	bge	148 <memcpy+0x148>
 1a4:	e8bd03e0 	pop	{r5, r6, r7, r8, r9}
 1a8:	e212c01c 	ands	ip, r2, #28
 1ac:	0a000005 	beq	1c8 <memcpy+0x1c8>
 1b0:	e1a0342e 	lsr	r3, lr, #8
 1b4:	e491e004 	ldr	lr, [r1], #4
 1b8:	e25cc004 	subs	ip, ip, #4
 1bc:	e1833c0e 	orr	r3, r3, lr, lsl #24
 1c0:	e4803004 	str	r3, [r0], #4
 1c4:	cafffff9 	bgt	1b0 <memcpy+0x1b0>
 1c8:	e2411003 	sub	r1, r1, #3
 1cc:	eaffffba 	b	bc <memcpy+0xbc>

17:		forward_copy_shift	pull=16	push=16
 1d0:	e252201c 	subs	r2, r2, #28
 1d4:	ba00001f 	blt	258 <memcpy+0x258>
 1d8:	e92d03e0 	push	{r5, r6, r7, r8, r9}
 1dc:	f5d1f000 	pld	[r1]
 1e0:	e2522060 	subs	r2, r2, #96	; 0x60
 1e4:	f5d1f01c 	pld	[r1, #28]
 1e8:	ba000002 	blt	1f8 <memcpy+0x1f8>
 1ec:	f5d1f03c 	pld	[r1, #60]	; 0x3c
 1f0:	f5d1f05c 	pld	[r1, #92]	; 0x5c
 1f4:	f5d1f07c 	pld	[r1, #124]	; 0x7c
 1f8:	e8b100f0 	ldm	r1!, {r4, r5, r6, r7}
 1fc:	e1a0382e 	lsr	r3, lr, #16
 200:	e2522020 	subs	r2, r2, #32
 204:	e8b15300 	ldm	r1!, {r8, r9, ip, lr}
 208:	e1833804 	orr	r3, r3, r4, lsl #16
 20c:	e1a04824 	lsr	r4, r4, #16
 210:	e1844805 	orr	r4, r4, r5, lsl #16
 214:	e1a05825 	lsr	r5, r5, #16
 218:	e1855806 	orr	r5, r5, r6, lsl #16
 21c:	e1a06826 	lsr	r6, r6, #16
 220:	e1866807 	orr	r6, r6, r7, lsl #16
 224:	e1a07827 	lsr	r7, r7, #16
 228:	e1877808 	orr	r7, r7, r8, lsl #16
 22c:	e1a08828 	lsr	r8, r8, #16
 230:	e1888809 	orr	r8, r8, r9, lsl #16
 234:	e1a09829 	lsr	r9, r9, #16
 238:	e189980c 	orr	r9, r9, ip, lsl #16
 23c:	e1a0c82c 	lsr	ip, ip, #16
 240:	e18cc80e 	orr	ip, ip, lr, lsl #16
 244:	e8a013f8 	stmia	r0!, {r3, r4, r5, r6, r7, r8, r9, ip}
 248:	aaffffe9 	bge	1f4 <memcpy+0x1f4>
 24c:	e3720060 	cmn	r2, #96	; 0x60
 250:	aaffffe8 	bge	1f8 <memcpy+0x1f8>
 254:	e8bd03e0 	pop	{r5, r6, r7, r8, r9}
 258:	e212c01c 	ands	ip, r2, #28
 25c:	0a000005 	beq	278 <memcpy+0x278>
 260:	e1a0382e 	lsr	r3, lr, #16
 264:	e491e004 	ldr	lr, [r1], #4
 268:	e25cc004 	subs	ip, ip, #4
 26c:	e183380e 	orr	r3, r3, lr, lsl #16
 270:	e4803004 	str	r3, [r0], #4
 274:	cafffff9 	bgt	260 <memcpy+0x260>
 278:	e2411002 	sub	r1, r1, #2
 27c:	eaffff8e 	b	bc <memcpy+0xbc>

18:		forward_copy_shift	pull=24	push=8
 280:	e252201c 	subs	r2, r2, #28
 284:	ba00001f 	blt	308 <memcpy+0x308>
 288:	e92d03e0 	push	{r5, r6, r7, r8, r9}
 28c:	f5d1f000 	pld	[r1]
 290:	e2522060 	subs	r2, r2, #96	; 0x60
 294:	f5d1f01c 	pld	[r1, #28]
 298:	ba000002 	blt	2a8 <memcpy+0x2a8>
 29c:	f5d1f03c 	pld	[r1, #60]	; 0x3c
 2a0:	f5d1f05c 	pld	[r1, #92]	; 0x5c
 2a4:	f5d1f07c 	pld	[r1, #124]	; 0x7c
 2a8:	e8b100f0 	ldm	r1!, {r4, r5, r6, r7}
 2ac:	e1a03c2e 	lsr	r3, lr, #24
 2b0:	e2522020 	subs	r2, r2, #32
 2b4:	e8b15300 	ldm	r1!, {r8, r9, ip, lr}
 2b8:	e1833404 	orr	r3, r3, r4, lsl #8
 2bc:	e1a04c24 	lsr	r4, r4, #24
 2c0:	e1844405 	orr	r4, r4, r5, lsl #8
 2c4:	e1a05c25 	lsr	r5, r5, #24
 2c8:	e1855406 	orr	r5, r5, r6, lsl #8
 2cc:	e1a06c26 	lsr	r6, r6, #24
 2d0:	e1866407 	orr	r6, r6, r7, lsl #8
 2d4:	e1a07c27 	lsr	r7, r7, #24
 2d8:	e1877408 	orr	r7, r7, r8, lsl #8
 2dc:	e1a08c28 	lsr	r8, r8, #24
 2e0:	e1888409 	orr	r8, r8, r9, lsl #8
 2e4:	e1a09c29 	lsr	r9, r9, #24
 2e8:	e189940c 	orr	r9, r9, ip, lsl #8
 2ec:	e1a0cc2c 	lsr	ip, ip, #24
 2f0:	e18cc40e 	orr	ip, ip, lr, lsl #8
 2f4:	e8a013f8 	stmia	r0!, {r3, r4, r5, r6, r7, r8, r9, ip}
 2f8:	aaffffe9 	bge	2a4 <memcpy+0x2a4>
 2fc:	e3720060 	cmn	r2, #96	; 0x60
 300:	aaffffe8 	bge	2a8 <memcpy+0x2a8>
 304:	e8bd03e0 	pop	{r5, r6, r7, r8, r9}
 308:	e212c01c 	ands	ip, r2, #28
 30c:	0a000005 	beq	328 <memcpy+0x328>
 310:	e1a03c2e 	lsr	r3, lr, #24
 314:	e491e004 	ldr	lr, [r1], #4
 318:	e25cc004 	subs	ip, ip, #4
 31c:	e183340e 	orr	r3, r3, lr, lsl #8
 320:	e4803004 	str	r3, [r0], #4
 324:	cafffff9 	bgt	310 <memcpy+0x310>
 328:	e2411001 	sub	r1, r1, #1
 32c:	eaffff62 	b	bc <memcpy+0xbc>

[-- Attachment #3: Type: text/plain, Size: 144 bytes --]

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/