Kernel crashes when CONFIG_MTD_NAND_VERIFY_WRITE=y

Kernel crashes when CONFIG_MTD_NAND_VERIFY_WRITE=y

[Fabio Estevam]

Hi,

When enabling CONFIG_MTD_NAND_VERIFY_WRITE=y on a mx28evk board
running 3.6-rc1  I get the following crash:

root@freescale /$ dd if=/dev/zero of=/dev/mtd0 bs=128k count=1
[   41.070000] Unable to handle kernel NULL pointer dereference at virtual addr0
[   41.080000] pgd = c77ac000
[   41.080000] [00000000] *pgd=4773c831, *pte=00000000, *ppte=00000000
[   41.090000] Internal error: Oops: 17 [#1] ARM
[   41.090000] Modules linked in:
[   41.090000] CPU: 0    Not tainted  (3.6.0-rc1-next-20120809-00002-ga25d017-d)
[   41.090000] PC is at nand_verify_buf+0x18/0x5c
[   41.090000] LR is at nand_write_page+0xc0/0x140
[   41.090000] pc : [<c023e7d4>]    lr : [<c023fcfc>]    psr: 20000013
[   41.090000] sp : c7779de4  ip : 00000000  fp : c6e60000
[   41.090000] r10: 00000000  r9 : 00020000  r8 : 00000000
[   41.090000] r7 : 00000000  r6 : c6e60000  r5 : c762cb10  r4 : 00000000
[   41.090000] r3 : c762c8a8  r2 : 00000800  r1 : c6e60000  r0 : 00000000
[   41.090000] Flags: nzCv  IRQs on  FIQs on  Mode SVC_32  ISA ARM  Segment user
[   41.090000] Control: 0005317f  Table: 477ac000  DAC: 00000015
[   41.090000] Process dd (pid: 427, stack limit = 0xc7778270)
[   41.090000] Stack: (0xc7779de4 to 0xc777a000)
[   41.090000] 9de0:          c762c8a8 c762c8a8 c0246ba8 00000800 c762cb10 c7620
[   41.090000] 9e00: 00000001 c0240c98 00000000 00000001 00000000 00000007 c7620
[   41.090000] 9e20: 00000000 c6e60000 00000000 ffffffff 00000000 00020000 00000
[   41.090000] 9e40: 0000007f 00000000 c77cc400 c004a934 00000000 c762cb10 c7620
[   41.090000] 9e60: 00000000 80000000 00000000 80000000 00000000 c0241010 c7775
[   41.090000] 9e80: 00000000 00020000 00000000 00020000 00000000 00000000 c6e60
[   41.090000] 9ea0: 00020000 00000400 00000000 c0238368 00020000 c7779f28 c6e60
[   41.090000] 9ec0: 00020000 c7779f80 c77c7ae0 b6dde008 c7778000 c7779f28 c6e68
[   41.090000] 9ee0: 00020000 c7779f28 c6e60000 00000005 c04f46dc 00000000 c7770
[   41.090000] 9f00: b6dde008 c6e60000 00000000 c00085a0 c7778000 c7688cc8 00000
[   41.090000] 9f20: 00000000 00000000 00000000 00020000 00000000 c77b7f00 00028
[   41.090000] 9f40: c7779f80 c7778000 00000000 00000000 00000000 c00cc054 00000
[   41.090000] 9f60: c000f298 c77b7f00 b6dde008 00020000 00000004 00000000 0000c
[   41.090000] 9f80: 00000000 00000000 b6f4d4d0 00000000 00020000 b6dde008 b6f48
[   41.090000] 9fa0: c7778000 c000f1a0 00020000 b6dde008 00000001 b6dde008 0002c
[   41.090000] 9fc0: 00020000 b6dde008 b6f4d4d0 00000004 00098a30 b6dde008 00000
[   41.090000] 9fe0: 00000001 bebc5b68 0000f338 b6eb60bc 60000010 00000001 00000
[   41.090000] [<c023e7d4>] (nand_verify_buf+0x18/0x5c) from [<c762c8a8>] (0xc7)
[   41.090000] Code: e59030dc da00000d e5934000 e5d10000 (e5d43000)
[   41.320000] ---[ end trace 92fce7df493d29ed ]---

The same write command works if CONFIG_MTD_NAND_VERIFY_WRITE is unselected.

Any ideas?

Thanks,

Fabio Estevam

Re: Kernel crashes when CONFIG_MTD_NAND_VERIFY_WRITE=y

[Marek Vasut]

Dear Fabio Estevam,

> Hi,
> 
> When enabling CONFIG_MTD_NAND_VERIFY_WRITE=y on a mx28evk board
> running 3.6-rc1  I get the following crash:
> 
> root@freescale /$ dd if=/dev/zero of=/dev/mtd0 bs=128k count=1
> [   41.070000] Unable to handle kernel NULL pointer dereference at virtual
[...]

This problem is there because the GPMI NAND code doesn't implement verify buffer 
function and defaults to nand_verify_buf() call in nand_base.c:

 253 static int nand_verify_buf(struct mtd_info *mtd, const uint8_t *buf, int 
len)
 254 {
 255         int i;
 256         struct nand_chip *chip = mtd->priv;
 257 
 258         for (i = 0; i < len; i++)
 259                 if (buf[i] != readb(chip->IO_ADDR_R))
 260                         return -EFAULT;
 261         return 0;
 262 }

Now the chip->IO_ADDR_R is zero, making the kernel access bogus location, and 
therefore crash. So the correct solution is to properly implement the struct 
nand_chip *'s verify_buf function.

Best regards,
Marek Vasut

Re: Kernel crashes when CONFIG_MTD_NAND_VERIFY_WRITE=y

[Fabio Estevam]

Hi Marek,

On Thu, Aug 9, 2012 at 8:53 PM, Marek Vasut <marex@denx.de> wrote:

> This problem is there because the GPMI NAND code doesn't implement verify buffer
> function and defaults to nand_verify_buf() call in nand_base.c:

Yes, you are right.

> Now the chip->IO_ADDR_R is zero, making the kernel access bogus location, and
> therefore crash. So the correct solution is to properly implement the struct
> nand_chip *'s verify_buf function.

Right, the patch below prevents the kernel to happen:

--- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
+++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
@@ -857,6 +857,15 @@ static uint8_t gpmi_read_byte(struct mtd_info *mtd)
        return buf[0];
 }

+/* Used by the upper layer to verify the data in NAND Flash
+ * with the data in the buf. */
+static int gpmi_verify_buf(struct mtd_info *mtd,
+                               const u_char *buf, int len)
+{
+       /* TODO: implement verify_buf mechanism */
+       return 0;
+}
+
 /*
  * Handles block mark swapping.
  * It can be called in swapping the block mark, or swapping it back,
@@ -1568,6 +1577,7 @@ static int __devinit gpmi_nfc_init(struct
gpmi_nand_data *this)
        chip->ecc.size          = 1;
        chip->ecc.strength      = 8;
        chip->ecc.layout        = &gpmi_hw_ecclayout;
+       chip->verify_buf        = gpmi_verify_buf;
        if (of_get_nand_on_flash_bbt(this->dev->of_node))
                chip->bbt_options |= NAND_BBT_USE_FLASH | NAND_BBT_NO_OOB;

Now we need to come up with a real  gpmi_verify_buf function ;-)

Regards,

Fabio Estevam

Re: Kernel crashes when CONFIG_MTD_NAND_VERIFY_WRITE=y

[Marek Vasut]

Dear Fabio Estevam,

> Hi Marek,
> 
> On Thu, Aug 9, 2012 at 8:53 PM, Marek Vasut <marex@denx.de> wrote:
> > This problem is there because the GPMI NAND code doesn't implement verify
> > buffer
> 
> > function and defaults to nand_verify_buf() call in nand_base.c:
> Yes, you are right.
> 
> > Now the chip->IO_ADDR_R is zero, making the kernel access bogus location,
> > and therefore crash. So the correct solution is to properly implement
> > the struct nand_chip *'s verify_buf function.
> 
> Right, the patch below prevents the kernel to happen:
> 
> --- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> +++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> @@ -857,6 +857,15 @@ static uint8_t gpmi_read_byte(struct mtd_info *mtd)
>         return buf[0];
>  }
> 
> +/* Used by the upper layer to verify the data in NAND Flash
> + * with the data in the buf. */
> +static int gpmi_verify_buf(struct mtd_info *mtd,
> +                               const u_char *buf, int len)
> +{
> +       /* TODO: implement verify_buf mechanism */
> +       return 0;
> +}


NAK! This is only a workaround, proper implementation is needed. If it's not 
implemented now, I'm pretty sure such workaround will be there forever.

>  /*
>   * Handles block mark swapping.
>   * It can be called in swapping the block mark, or swapping it back,
> @@ -1568,6 +1577,7 @@ static int __devinit gpmi_nfc_init(struct
> gpmi_nand_data *this)
>         chip->ecc.size          = 1;
>         chip->ecc.strength      = 8;
>         chip->ecc.layout        = &gpmi_hw_ecclayout;
> +       chip->verify_buf        = gpmi_verify_buf;
>         if (of_get_nand_on_flash_bbt(this->dev->of_node))
>                 chip->bbt_options |= NAND_BBT_USE_FLASH | NAND_BBT_NO_OOB;
> 
> Now we need to come up with a real  gpmi_verify_buf function ;-)
> 
> Regards,
> 
> Fabio Estevam

Best regards,
Marek Vasut

Re: Kernel crashes when CONFIG_MTD_NAND_VERIFY_WRITE=y

[Marek Vasut]

Dear Marek Vasut,

> Dear Fabio Estevam,
> 
> > Hi Marek,
> > 
> > On Thu, Aug 9, 2012 at 8:53 PM, Marek Vasut <marex@denx.de> wrote:
> > > This problem is there because the GPMI NAND code doesn't implement
> > > verify buffer
> > 
> > > function and defaults to nand_verify_buf() call in nand_base.c:
> > Yes, you are right.
> > 
> > > Now the chip->IO_ADDR_R is zero, making the kernel access bogus
> > > location, and therefore crash. So the correct solution is to properly
> > > implement the struct nand_chip *'s verify_buf function.
> > 
> > Right, the patch below prevents the kernel to happen:
> > 
> > --- a/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> > +++ b/drivers/mtd/nand/gpmi-nand/gpmi-nand.c
> > @@ -857,6 +857,15 @@ static uint8_t gpmi_read_byte(struct mtd_info *mtd)
> > 
> >         return buf[0];
> >  
> >  }
> > 
> > +/* Used by the upper layer to verify the data in NAND Flash
> > + * with the data in the buf. */
> > +static int gpmi_verify_buf(struct mtd_info *mtd,
> > +                               const u_char *buf, int len)
> > +{
> > +       /* TODO: implement verify_buf mechanism */
> > +       return 0;
> > +}
> 
> NAK! This is only a workaround, proper implementation is needed. If it's
> not implemented now, I'm pretty sure such workaround will be there
> forever.
[...]

btw if you want a workaround, make Kconfig entry so it'd disallow VERIFY to be 
selected for GPMI_NAND, but that's also a wrong way to go.

Best regards,
Marek Vasut

Re: Kernel crashes when CONFIG_MTD_NAND_VERIFY_WRITE=y

[Fabio Estevam]

On Thu, Aug 9, 2012 at 10:41 PM, Marek Vasut <marex@denx.de> wrote:

> NAK! This is only a workaround, proper implementation is needed. If it's not
> implemented now, I'm pretty sure such workaround will be there forever.

I know, please see below.

>
>>  /*
>>   * Handles block mark swapping.
>>   * It can be called in swapping the block mark, or swapping it back,
>> @@ -1568,6 +1577,7 @@ static int __devinit gpmi_nfc_init(struct
>> gpmi_nand_data *this)
>>         chip->ecc.size          = 1;
>>         chip->ecc.strength      = 8;
>>         chip->ecc.layout        = &gpmi_hw_ecclayout;
>> +       chip->verify_buf        = gpmi_verify_buf;
>>         if (of_get_nand_on_flash_bbt(this->dev->of_node))
>>                 chip->bbt_options |= NAND_BBT_USE_FLASH | NAND_BBT_NO_OOB;
>>
>> Now we need to come up with a real  gpmi_verify_buf function ;-)

As I mentioned, I understand that a proper function needs to be created.

Regards,

Fabio Estevam

Re: Kernel crashes when CONFIG_MTD_NAND_VERIFY_WRITE=y

[Huang Shijie]

于 2012年08月10日 09:49, Fabio Estevam 写道:
> On Thu, Aug 9, 2012 at 10:41 PM, Marek Vasut<marex@denx.de>  wrote:
>
>> NAK! This is only a workaround, proper implementation is needed. If it's not
>> implemented now, I'm pretty sure such workaround will be there forever.
> I know, please see below.
>
>>>   /*
>>>    * Handles block mark swapping.
>>>    * It can be called in swapping the block mark, or swapping it back,
>>> @@ -1568,6 +1577,7 @@ static int __devinit gpmi_nfc_init(struct
>>> gpmi_nand_data *this)
>>>          chip->ecc.size          = 1;
>>>          chip->ecc.strength      = 8;
>>>          chip->ecc.layout        =&gpmi_hw_ecclayout;
>>> +       chip->verify_buf        = gpmi_verify_buf;
>>>          if (of_get_nand_on_flash_bbt(this->dev->of_node))
>>>                  chip->bbt_options |= NAND_BBT_USE_FLASH | NAND_BBT_NO_OOB;
>>>
>>> Now we need to come up with a real  gpmi_verify_buf function ;-)
I does have a real gpmi_verify_buf function in our BSP code.

I will send it out as soon as possible.

thanks
Huang Shijie




> As I mentioned, I understand that a proper function needs to be created.
>
> Regards,
>
> Fabio Estevam
>

Re: Kernel crashes when CONFIG_MTD_NAND_VERIFY_WRITE=y

[Marek Vasut]

Dear Huang Shijie,

> 于 2012年08月10日 09:49, Fabio Estevam 写道:
> > On Thu, Aug 9, 2012 at 10:41 PM, Marek Vasut<marex@denx.de>  wrote:
> >> NAK! This is only a workaround, proper implementation is needed. If it's
> >> not implemented now, I'm pretty sure such workaround will be there
> >> forever.
> > 
> > I know, please see below.
> > 
> >>>   /*
> >>>   
> >>>    * Handles block mark swapping.
> >>>    * It can be called in swapping the block mark, or swapping it back,
> >>> 
> >>> @@ -1568,6 +1577,7 @@ static int __devinit gpmi_nfc_init(struct
> >>> gpmi_nand_data *this)
> >>> 
> >>>          chip->ecc.size          = 1;
> >>>          chip->ecc.strength      = 8;
> >>>          chip->ecc.layout        =&gpmi_hw_ecclayout;
> >>> 
> >>> +       chip->verify_buf        = gpmi_verify_buf;
> >>> 
> >>>          if (of_get_nand_on_flash_bbt(this->dev->of_node))
> >>>          
> >>>                  chip->bbt_options |= NAND_BBT_USE_FLASH |
> >>>                  NAND_BBT_NO_OOB;
> >>> 
> >>> Now we need to come up with a real  gpmi_verify_buf function ;-)
> 
> I does have a real gpmi_verify_buf function in our BSP code.
> 
> I will send it out as soon as possible.

I believe you should roll it out ASAP and also CC stable!

> thanks
> Huang Shijie
> 
> > As I mentioned, I understand that a proper function needs to be created.
> > 
> > Regards,
> > 
> > Fabio Estevam

Best regards,
Marek Vasut

Re: Kernel crashes when CONFIG_MTD_NAND_VERIFY_WRITE=y

[Fabio Estevam]

On Thu, Aug 9, 2012 at 11:08 PM, Huang Shijie <b32955@freescale.com> wrote:

> I does have a real gpmi_verify_buf function in our BSP code.

Well, the same crash happens in FSL BSP.

> I will send it out as soon as possible.

Ok, great.

Thanks,

Fabio Estevam