diff mbox series

mtd: spear_smi: Fix nonalignment not handled in memcpy_toio

Message ID 20191018143643.29676-1-miquel.raynal@bootlin.com
State Changes Requested
Delegated to: Miquel Raynal
Headers show
Series mtd: spear_smi: Fix nonalignment not handled in memcpy_toio | expand

Commit Message

Miquel Raynal Oct. 18, 2019, 2:36 p.m. UTC
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(-)

Comments

Miquel Raynal Oct. 18, 2019, 2:49 p.m. UTC | #1
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
Boris Brezillon Oct. 21, 2019, 8:01 a.m. UTC | #2
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
Miquel Raynal Oct. 22, 2019, 7:44 a.m. UTC | #3
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
Miquel Raynal Oct. 22, 2019, 7:51 a.m. UTC | #4
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
Thomas Petazzoni Oct. 22, 2019, 7:52 a.m. UTC | #5
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
Russell King (Oracle) Oct. 22, 2019, 8:26 a.m. UTC | #6
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
Miquel Raynal Oct. 22, 2019, 9:17 a.m. UTC | #7
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
Russell King (Oracle) Oct. 22, 2019, 9:26 a.m. UTC | #8
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
>
Miquel Raynal Oct. 22, 2019, 9:47 a.m. UTC | #9
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
diff mbox series

Patch

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