linux-mtd.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] mtd: spear_smi: Fix nonalignment not handled in memcpy_toio
@ 2019-10-18 14:36 Miquel Raynal
  2019-10-18 14:49 ` Miquel Raynal
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Miquel Raynal @ 2019-10-18 14:36 UTC (permalink / raw)
  To: Richard Weinberger, David Woodhouse, Brian Norris, Marek Vasut,
	Tudor Ambarus, Vignesh Raghavendra
  Cc: stable, Boris Brezillon, linux-mtd, Thomas Petazzoni,
	Miquel Raynal, linux-arm-kernel

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

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>
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);
 
-- 
2.20.1


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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] mtd: spear_smi: Fix nonalignment not handled in memcpy_toio
  2019-10-18 14:36 [PATCH] mtd: spear_smi: Fix nonalignment not handled in memcpy_toio Miquel Raynal
@ 2019-10-18 14:49 ` Miquel Raynal
  2019-10-21  8:01 ` Boris Brezillon
  2019-10-22  8:26 ` Russell King - ARM Linux admin
  2 siblings, 0 replies; 10+ messages in thread
From: Miquel Raynal @ 2019-10-18 14:49 UTC (permalink / raw)
  To: Richard Weinberger, David Woodhouse, Brian Norris, Marek Vasut,
	Tudor Ambarus, Vignesh Raghavendra
  Cc: stable, Boris Brezillon, linux-mtd, Thomas Petazzoni, rmk+kernel,
	linux-arm-kernel

Hello,

+Russell who might have thoughts about the issue

Miquel Raynal <miquel.raynal@bootlin.com> wrote on Fri, 18 Oct 2019
16:36:43 +0200:

> 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
> 
> 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>
> 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.

I did not mention that the opposite direction using memcpy_fromio() does
not present any issue.

> 
> 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);
>  




Thanks,
Miquèl

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] mtd: spear_smi: Fix nonalignment not handled in memcpy_toio
  2019-10-18 14:36 [PATCH] mtd: spear_smi: Fix nonalignment not handled in memcpy_toio Miquel Raynal
  2019-10-18 14:49 ` Miquel Raynal
@ 2019-10-21  8:01 ` Boris Brezillon
  2019-10-22  7:44   ` Miquel Raynal
  2019-10-22  8:26 ` Russell King - ARM Linux admin
  2 siblings, 1 reply; 10+ messages in thread
From: Boris Brezillon @ 2019-10-21  8:01 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: Vignesh Raghavendra, Tudor Ambarus, Richard Weinberger, stable,
	Marek Vasut, linux-mtd, Thomas Petazzoni, Brian Norris,
	David Woodhouse, linux-arm-kernel

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].

> 
> 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

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] mtd: spear_smi: Fix nonalignment not handled in memcpy_toio
  2019-10-21  8:01 ` Boris Brezillon
@ 2019-10-22  7:44   ` Miquel Raynal
  2019-10-22  7:51     ` Miquel Raynal
  2019-10-22  7:52     ` Thomas Petazzoni
  0 siblings, 2 replies; 10+ messages in thread
From: Miquel Raynal @ 2019-10-22  7:44 UTC (permalink / raw)
  To: Boris Brezillon
  Cc: Vignesh Raghavendra, Tudor Ambarus, Richard Weinberger, stable,
	Marek Vasut, linux-mtd, Thomas Petazzoni, Brian Norris,
	David Woodhouse, linux-arm-kernel

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

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/

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] mtd: spear_smi: Fix nonalignment not handled in memcpy_toio
  2019-10-22  7:44   ` Miquel Raynal
@ 2019-10-22  7:51     ` Miquel Raynal
  2019-10-22  7:52     ` Thomas Petazzoni
  1 sibling, 0 replies; 10+ messages in thread
From: Miquel Raynal @ 2019-10-22  7:51 UTC (permalink / raw)
  To: Boris Brezillon
  Cc: Vignesh Raghavendra, Tudor Ambarus, Richard Weinberger,
	Russell King, stable, Marek Vasut, linux-mtd, Thomas Petazzoni,
	Brian Norris, David Woodhouse, linux-arm-kernel

[-- 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/

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] mtd: spear_smi: Fix nonalignment not handled in memcpy_toio
  2019-10-22  7:44   ` Miquel Raynal
  2019-10-22  7:51     ` Miquel Raynal
@ 2019-10-22  7:52     ` Thomas Petazzoni
  1 sibling, 0 replies; 10+ messages in thread
From: Thomas Petazzoni @ 2019-10-22  7:52 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: Vignesh Raghavendra, Tudor Ambarus, Richard Weinberger,
	Russell King - ARM Linux admin, stable, Marek Vasut,
	Boris Brezillon, linux-mtd, Brian Norris, David Woodhouse,
	linux-arm-kernel

Hello,

+Russell in Cc.

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

> 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.

What I find odd is:

 (1) Failing on a 1-byte store instruction, which means it should have
     no alignment constraints.

 (2) Failing on a 1-byte store instruction, while switching to
     _memcpy_toio(), which does *only* 1-byte stores, works around the
     problem.

_memcpy_toio() looks like this:

void _memcpy_toio(volatile void __iomem *to, const void *from, size_t count)
{
	const unsigned char *f = from;
	while (count) {
  6c:	e3520000 	cmp	r2, #0
  70:	012fff1e 	bxeq	lr
  74:	e0802002 	add	r2, r0, r2
		count--;
		writeb(*f, to);
  78:	e4d13001 	ldrb	r3, [r1], #1
	asm volatile("strb %1, %0"
  7c:	e5c03000 	strb	r3, [r0]
		f++;
		to++;
  80:	e2800001 	add	r0, r0, #1
	while (count) {
  84:	e1500002 	cmp	r0, r2
  88:	1afffffa 	bne	78 <_memcpy_toio+0xc>
  8c:	e12fff1e 	bx	lr

So it's also doing a strb, nothing different.

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] mtd: spear_smi: Fix nonalignment not handled in memcpy_toio
  2019-10-18 14:36 [PATCH] mtd: spear_smi: Fix nonalignment not handled in memcpy_toio Miquel Raynal
  2019-10-18 14:49 ` Miquel Raynal
  2019-10-21  8:01 ` Boris Brezillon
@ 2019-10-22  8:26 ` Russell King - ARM Linux admin
  2019-10-22  9:17   ` Miquel Raynal
  2 siblings, 1 reply; 10+ messages in thread
From: Russell King - ARM Linux admin @ 2019-10-22  8:26 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: Vignesh Raghavendra, Tudor Ambarus, Richard Weinberger, stable,
	Marek Vasut, Boris Brezillon, linux-mtd, Thomas Petazzoni,
	Brian Norris, David Woodhouse, linux-arm-kernel

On Fri, Oct 18, 2019 at 04:36:43PM +0200, Miquel Raynal 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

I need the full oops if you want me to comment on this.

> 
> 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>
> 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);
>  
> -- 
> 2.20.1
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] mtd: spear_smi: Fix nonalignment not handled in memcpy_toio
  2019-10-22  8:26 ` Russell King - ARM Linux admin
@ 2019-10-22  9:17   ` Miquel Raynal
  2019-10-22  9:26     ` Russell King - ARM Linux admin
  0 siblings, 1 reply; 10+ messages in thread
From: Miquel Raynal @ 2019-10-22  9:17 UTC (permalink / raw)
  To: Russell King - ARM Linux admin
  Cc: Vignesh Raghavendra, Tudor Ambarus, Richard Weinberger, stable,
	Marek Vasut, Boris Brezillon, linux-mtd, Thomas Petazzoni,
	Brian Norris, David Woodhouse, linux-arm-kernel

Hi Russell,

Russell King - ARM Linux admin <linux@armlinux.org.uk> wrote on Tue, 22
Oct 2019 09:26:43 +0100:

> On Fri, Oct 18, 2019 at 04:36:43PM +0200, Miquel Raynal 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  
> 
> I need the full oops if you want me to comment on this.

FYI, I ran the dd command within a for loop, incrementing the block size
(bs) by one byte, if failed with bs=6.

Disabling WB_MODE (burst mode) does not change anything.

Adding a wmb() right after the memcpy_toio() prevents the fault.

Here is the full trace when writing 1001 bytes:

# dd if=/dev/urandom bs=1001 count=1 of=/dev/mtd6
Unhandled fault: external abort on non-linefetch (0x808) at 0xc90703e8
pgd = c7be8000
[c90703e8] *pgd=f8000452(bad)
Internal error: : 808 [#1] ARM
Modules linked in:
CPU: 0 PID: 660 Comm: dd Not tainted 4.14.0-00045-gf5d08192704f-dirty #6
Hardware name: ST SPEAr600 (Flattened Device Tree)
task: c7a05080 task.stack: c7bd2000
PC is at memcpy+0xcc/0x330
LR is at 0x13f0ec28
pc : [<c044344c>]    lr : [<13f0ec28>]    psr: 80000013
sp : c7bd3e44  ip : 00000018  fp : 000003e9
r10: 00000000  r9 : c7a9959c  r8 : c7bd3eac
r7 : c7a99590  r6 : c7afb438  r5 : 00000300  r4 : 5171436c
r3 : 00000058  r2 : 80000000  r1 : c7be4be9  r0 : c90703e8
Flags: Nzcv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment none
Control: 0005317f  Table: 07be8000  DAC: 00000051
Process dd (pid: 660, stack limit = 0xc7bd2190)
Stack: (0xc7bd3e44 to 0xc7bd4000)
3e40:          c9070300 000000e9 c0290d14 c9070300 c7be4b00 0001046f c9070000
3e60: c7afb418 00000000 c7bd3e98 000003e9 c7bd3f88 000003e9 00000000 000c0008
3e80: 00000051 c7bd2000 c7be4800 c028e57c 000003e9 c7bd3eac c7be4800 00000000
3ea0: c7bf73c0 c7addc00 000003e9 00000300 00000000 00000000 00000000 00000000
3ec0: 00000000 00000000 00000000 00000000 00000000 000003e9 c028e4bc c7962a80
3ee0: c7bd3f88 00000000 c7bd2000 00000000 000bf990 c00bdb0c 000bf990 c00bd878
3f00: 00000000 00000000 00000000 c7bd3f10 c7a688c0 c009eedc c7becb58 000c0000
3f20: 00000003 c7962460 c7962484 00000000 00000000 c045c2f8 00000003 c00d9e58
3f40: 000003e9 000c0008 c7962a80 c7bd3f88 00000000 c7bd2000 00000000 c00bddb4
3f60: 000bf990 c00bda6c 00000000 c7962a80 c7962a80 000c0008 000003e9 c000a804
3f80: c7bd2000 c00bdfa4 00000000 00000000 00000000 000bfd94 00000001 000c0008
3fa0: 00000004 c000a640 000bfd94 00000001 00000001 000c0008 000003e9 be8e8f53
3fc0: 000bfd94 00000001 000c0008 00000004 000c0008 000c0008 000003e9 000bf990
3fe0: 00000000 be8e8ba4 0000ea3c b6eba7ec 60000010 00000001 00000000 00000000
[<c044344c>] (memcpy) from [<c0290d14>] (spear_mtd_write+0x240/0x294)
[<c0290d14>] (spear_mtd_write) from [<c028e57c>] (mtdchar_write+0xc0/0x230)
[<c028e57c>] (mtdchar_write) from [<c00bdb0c>] (__vfs_write+0x1c/0x128)
[<c00bdb0c>] (__vfs_write) from [<c00bddb4>] (vfs_write+0xa0/0x168)
[<c00bddb4>] (vfs_write) from [<c00bdfa4>] (SyS_write+0x3c/0x90)
[<c00bdfa4>] (SyS_write) from [<c000a640>] (ret_fast_syscall+0x0/0x44)
Code: e1b02f82 14d13001 24d14001 24d1c001 (14c03001) 
---[ end trace f9a736cc2841cf14 ]---
Segmentation fault


Thanks,
Miquèl

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] mtd: spear_smi: Fix nonalignment not handled in memcpy_toio
  2019-10-22  9:17   ` Miquel Raynal
@ 2019-10-22  9:26     ` Russell King - ARM Linux admin
  2019-10-22  9:47       ` Miquel Raynal
  0 siblings, 1 reply; 10+ messages in thread
From: Russell King - ARM Linux admin @ 2019-10-22  9:26 UTC (permalink / raw)
  To: Miquel Raynal
  Cc: Vignesh Raghavendra, Tudor Ambarus, Richard Weinberger, stable,
	Marek Vasut, Boris Brezillon, linux-mtd, Thomas Petazzoni,
	Brian Norris, David Woodhouse, linux-arm-kernel

On Tue, Oct 22, 2019 at 11:17:07AM +0200, Miquel Raynal wrote:
> Hi Russell,
> 
> Russell King - ARM Linux admin <linux@armlinux.org.uk> wrote on Tue, 22
> Oct 2019 09:26:43 +0100:
> 
> > On Fri, Oct 18, 2019 at 04:36:43PM +0200, Miquel Raynal 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  
> > 
> > I need the full oops if you want me to comment on this.
> 
> FYI, I ran the dd command within a for loop, incrementing the block size
> (bs) by one byte, if failed with bs=6.
> 
> Disabling WB_MODE (burst mode) does not change anything.
> 
> Adding a wmb() right after the memcpy_toio() prevents the fault.

Thanks.  Can you check what the result of the write buffer test earlier
in the kernel boot is?

CPU: Testing write buffer coherency: ...

?

Thanks.

> 
> Here is the full trace when writing 1001 bytes:
> 
> # dd if=/dev/urandom bs=1001 count=1 of=/dev/mtd6
> Unhandled fault: external abort on non-linefetch (0x808) at 0xc90703e8
> pgd = c7be8000
> [c90703e8] *pgd=f8000452(bad)
> Internal error: : 808 [#1] ARM
> Modules linked in:
> CPU: 0 PID: 660 Comm: dd Not tainted 4.14.0-00045-gf5d08192704f-dirty #6
> Hardware name: ST SPEAr600 (Flattened Device Tree)
> task: c7a05080 task.stack: c7bd2000
> PC is at memcpy+0xcc/0x330
> LR is at 0x13f0ec28
> pc : [<c044344c>]    lr : [<13f0ec28>]    psr: 80000013
> sp : c7bd3e44  ip : 00000018  fp : 000003e9
> r10: 00000000  r9 : c7a9959c  r8 : c7bd3eac
> r7 : c7a99590  r6 : c7afb438  r5 : 00000300  r4 : 5171436c
> r3 : 00000058  r2 : 80000000  r1 : c7be4be9  r0 : c90703e8
> Flags: Nzcv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment none
> Control: 0005317f  Table: 07be8000  DAC: 00000051
> Process dd (pid: 660, stack limit = 0xc7bd2190)
> Stack: (0xc7bd3e44 to 0xc7bd4000)
> 3e40:          c9070300 000000e9 c0290d14 c9070300 c7be4b00 0001046f c9070000
> 3e60: c7afb418 00000000 c7bd3e98 000003e9 c7bd3f88 000003e9 00000000 000c0008
> 3e80: 00000051 c7bd2000 c7be4800 c028e57c 000003e9 c7bd3eac c7be4800 00000000
> 3ea0: c7bf73c0 c7addc00 000003e9 00000300 00000000 00000000 00000000 00000000
> 3ec0: 00000000 00000000 00000000 00000000 00000000 000003e9 c028e4bc c7962a80
> 3ee0: c7bd3f88 00000000 c7bd2000 00000000 000bf990 c00bdb0c 000bf990 c00bd878
> 3f00: 00000000 00000000 00000000 c7bd3f10 c7a688c0 c009eedc c7becb58 000c0000
> 3f20: 00000003 c7962460 c7962484 00000000 00000000 c045c2f8 00000003 c00d9e58
> 3f40: 000003e9 000c0008 c7962a80 c7bd3f88 00000000 c7bd2000 00000000 c00bddb4
> 3f60: 000bf990 c00bda6c 00000000 c7962a80 c7962a80 000c0008 000003e9 c000a804
> 3f80: c7bd2000 c00bdfa4 00000000 00000000 00000000 000bfd94 00000001 000c0008
> 3fa0: 00000004 c000a640 000bfd94 00000001 00000001 000c0008 000003e9 be8e8f53
> 3fc0: 000bfd94 00000001 000c0008 00000004 000c0008 000c0008 000003e9 000bf990
> 3fe0: 00000000 be8e8ba4 0000ea3c b6eba7ec 60000010 00000001 00000000 00000000
> [<c044344c>] (memcpy) from [<c0290d14>] (spear_mtd_write+0x240/0x294)
> [<c0290d14>] (spear_mtd_write) from [<c028e57c>] (mtdchar_write+0xc0/0x230)
> [<c028e57c>] (mtdchar_write) from [<c00bdb0c>] (__vfs_write+0x1c/0x128)
> [<c00bdb0c>] (__vfs_write) from [<c00bddb4>] (vfs_write+0xa0/0x168)
> [<c00bddb4>] (vfs_write) from [<c00bdfa4>] (SyS_write+0x3c/0x90)
> [<c00bdfa4>] (SyS_write) from [<c000a640>] (ret_fast_syscall+0x0/0x44)
> Code: e1b02f82 14d13001 24d14001 24d1c001 (14c03001) 
> ---[ end trace f9a736cc2841cf14 ]---
> Segmentation fault
> 
> 
> Thanks,
> Miquèl
> 

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] mtd: spear_smi: Fix nonalignment not handled in memcpy_toio
  2019-10-22  9:26     ` Russell King - ARM Linux admin
@ 2019-10-22  9:47       ` Miquel Raynal
  0 siblings, 0 replies; 10+ messages in thread
From: Miquel Raynal @ 2019-10-22  9:47 UTC (permalink / raw)
  To: Russell King - ARM Linux admin
  Cc: Vignesh Raghavendra, Tudor Ambarus, Richard Weinberger, stable,
	Marek Vasut, Boris Brezillon, linux-mtd, Thomas Petazzoni,
	Brian Norris, David Woodhouse, linux-arm-kernel

Hi Russell,

Russell King - ARM Linux admin <linux@armlinux.org.uk> wrote on Tue, 22
Oct 2019 10:26:19 +0100:

> On Tue, Oct 22, 2019 at 11:17:07AM +0200, Miquel Raynal wrote:
> > Hi Russell,
> > 
> > Russell King - ARM Linux admin <linux@armlinux.org.uk> wrote on Tue, 22
> > Oct 2019 09:26:43 +0100:
> >   
> > > On Fri, Oct 18, 2019 at 04:36:43PM +0200, Miquel Raynal 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    
> > > 
> > > I need the full oops if you want me to comment on this.  
> > 
> > FYI, I ran the dd command within a for loop, incrementing the block size
> > (bs) by one byte, if failed with bs=6.
> > 
> > Disabling WB_MODE (burst mode) does not change anything.
> > 
> > Adding a wmb() right after the memcpy_toio() prevents the fault.  
> 
> Thanks.  Can you check what the result of the write buffer test earlier
> in the kernel boot is?
> 
> CPU: Testing write buffer coherency: ...
> 
> ?

CPU: Testing write buffer coherency: ok


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

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2019-10-22  9:48 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-18 14:36 [PATCH] mtd: spear_smi: Fix nonalignment not handled in memcpy_toio Miquel Raynal
2019-10-18 14:49 ` Miquel Raynal
2019-10-21  8:01 ` Boris Brezillon
2019-10-22  7:44   ` Miquel Raynal
2019-10-22  7:51     ` Miquel Raynal
2019-10-22  7:52     ` Thomas Petazzoni
2019-10-22  8:26 ` Russell King - ARM Linux admin
2019-10-22  9:17   ` Miquel Raynal
2019-10-22  9:26     ` Russell King - ARM Linux admin
2019-10-22  9:47       ` Miquel Raynal

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).