2019-09-07 17:45:13

by Kamal Dasu

[permalink] [raw]
Subject: [PATCH 2/2] mtd: rawnand: use bounce buffer when vmalloced data buf detected

For controller drivers that use DMA and set NAND_USE_BOUNCE_BUFFER
option use data buffers that are not vmalloced, aligned and have
valid virtual address to be able to do DMA transfers. This change
adds additional check and makes use of data buffer allocated
in nand_base driver when it is passed a vmalloced data buffer for
DMA transfers.

Signed-off-by: Kamal Dasu <[email protected]>
---
drivers/mtd/nand/raw/nand_base.c | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index 91f046d4d452..46f6965a896a 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -45,6 +45,12 @@

#include "internals.h"

+static int nand_need_bounce_buf(const void *buf, struct nand_chip *chip)
+{
+ return !virt_addr_valid(buf) || is_vmalloc_addr(buf) ||
+ !IS_ALIGNED((unsigned long)buf, chip->buf_align);
+}
+
/* Define default oob placement schemes for large and small page devices */
static int nand_ooblayout_ecc_sp(struct mtd_info *mtd, int section,
struct mtd_oob_region *oobregion)
@@ -3183,9 +3189,7 @@ static int nand_do_read_ops(struct nand_chip *chip, loff_t from,
if (!aligned)
use_bufpoi = 1;
else if (chip->options & NAND_USE_BOUNCE_BUFFER)
- use_bufpoi = !virt_addr_valid(buf) ||
- !IS_ALIGNED((unsigned long)buf,
- chip->buf_align);
+ use_bufpoi = nand_need_bounce_buf(buf, chip);
else
use_bufpoi = 0;

@@ -4009,9 +4013,7 @@ static int nand_do_write_ops(struct nand_chip *chip, loff_t to,
if (part_pagewr)
use_bufpoi = 1;
else if (chip->options & NAND_USE_BOUNCE_BUFFER)
- use_bufpoi = !virt_addr_valid(buf) ||
- !IS_ALIGNED((unsigned long)buf,
- chip->buf_align);
+ use_bufpoi = nand_need_bounce_buf(buf, chip);
else
use_bufpoi = 0;

--
2.17.1


2019-09-30 16:02:38

by Kamal Dasu

[permalink] [raw]
Subject: Re: [PATCH 2/2] mtd: rawnand: use bounce buffer when vmalloced data buf detected

Does anyone have any comments on this patch ?.

Kamal

On Fri, Sep 6, 2019 at 3:49 PM Kamal Dasu <[email protected]> wrote:
>
> For controller drivers that use DMA and set NAND_USE_BOUNCE_BUFFER
> option use data buffers that are not vmalloced, aligned and have
> valid virtual address to be able to do DMA transfers. This change
> adds additional check and makes use of data buffer allocated
> in nand_base driver when it is passed a vmalloced data buffer for
> DMA transfers.
>
> Signed-off-by: Kamal Dasu <[email protected]>
> ---
> drivers/mtd/nand/raw/nand_base.c | 14 ++++++++------
> 1 file changed, 8 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
> index 91f046d4d452..46f6965a896a 100644
> --- a/drivers/mtd/nand/raw/nand_base.c
> +++ b/drivers/mtd/nand/raw/nand_base.c
> @@ -45,6 +45,12 @@
>
> #include "internals.h"
>
> +static int nand_need_bounce_buf(const void *buf, struct nand_chip *chip)
> +{
> + return !virt_addr_valid(buf) || is_vmalloc_addr(buf) ||
> + !IS_ALIGNED((unsigned long)buf, chip->buf_align);
> +}
> +
> /* Define default oob placement schemes for large and small page devices */
> static int nand_ooblayout_ecc_sp(struct mtd_info *mtd, int section,
> struct mtd_oob_region *oobregion)
> @@ -3183,9 +3189,7 @@ static int nand_do_read_ops(struct nand_chip *chip, loff_t from,
> if (!aligned)
> use_bufpoi = 1;
> else if (chip->options & NAND_USE_BOUNCE_BUFFER)
> - use_bufpoi = !virt_addr_valid(buf) ||
> - !IS_ALIGNED((unsigned long)buf,
> - chip->buf_align);
> + use_bufpoi = nand_need_bounce_buf(buf, chip);
> else
> use_bufpoi = 0;
>
> @@ -4009,9 +4013,7 @@ static int nand_do_write_ops(struct nand_chip *chip, loff_t to,
> if (part_pagewr)
> use_bufpoi = 1;
> else if (chip->options & NAND_USE_BOUNCE_BUFFER)
> - use_bufpoi = !virt_addr_valid(buf) ||
> - !IS_ALIGNED((unsigned long)buf,
> - chip->buf_align);
> + use_bufpoi = nand_need_bounce_buf(buf, chip);
> else
> use_bufpoi = 0;
>
> --
> 2.17.1
>

2019-09-30 16:28:41

by Boris Brezillon

[permalink] [raw]
Subject: Re: [PATCH 2/2] mtd: rawnand: use bounce buffer when vmalloced data buf detected

On Mon, 30 Sep 2019 12:01:28 -0400
Kamal Dasu <[email protected]> wrote:

> Does anyone have any comments on this patch ?.
>
> Kamal
>
> On Fri, Sep 6, 2019 at 3:49 PM Kamal Dasu <[email protected]> wrote:
> >
> > For controller drivers that use DMA and set NAND_USE_BOUNCE_BUFFER
> > option use data buffers that are not vmalloced, aligned and have
> > valid virtual address to be able to do DMA transfers. This change
> > adds additional check and makes use of data buffer allocated
> > in nand_base driver when it is passed a vmalloced data buffer for
> > DMA transfers.
> >
> > Signed-off-by: Kamal Dasu <[email protected]>
> > ---
> > drivers/mtd/nand/raw/nand_base.c | 14 ++++++++------
> > 1 file changed, 8 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
> > index 91f046d4d452..46f6965a896a 100644
> > --- a/drivers/mtd/nand/raw/nand_base.c
> > +++ b/drivers/mtd/nand/raw/nand_base.c
> > @@ -45,6 +45,12 @@
> >
> > #include "internals.h"
> >
> > +static int nand_need_bounce_buf(const void *buf, struct nand_chip *chip)
> > +{
> > + return !virt_addr_valid(buf) || is_vmalloc_addr(buf) ||

I thought virt_addr_valid() was implying !is_vmalloc_addr(), are you
sure you need that test, and can you tell me on which arch(es) this is
needed.

> > + !IS_ALIGNED((unsigned long)buf, chip->buf_align);
> > +}
> > +
> > /* Define default oob placement schemes for large and small page devices */
> > static int nand_ooblayout_ecc_sp(struct mtd_info *mtd, int section,
> > struct mtd_oob_region *oobregion)
> > @@ -3183,9 +3189,7 @@ static int nand_do_read_ops(struct nand_chip *chip, loff_t from,
> > if (!aligned)
> > use_bufpoi = 1;
> > else if (chip->options & NAND_USE_BOUNCE_BUFFER)
> > - use_bufpoi = !virt_addr_valid(buf) ||
> > - !IS_ALIGNED((unsigned long)buf,
> > - chip->buf_align);
> > + use_bufpoi = nand_need_bounce_buf(buf, chip);
> > else
> > use_bufpoi = 0;
> >
> > @@ -4009,9 +4013,7 @@ static int nand_do_write_ops(struct nand_chip *chip, loff_t to,
> > if (part_pagewr)
> > use_bufpoi = 1;
> > else if (chip->options & NAND_USE_BOUNCE_BUFFER)
> > - use_bufpoi = !virt_addr_valid(buf) ||
> > - !IS_ALIGNED((unsigned long)buf,
> > - chip->buf_align);
> > + use_bufpoi = nand_need_bounce_buf(buf, chip);
> > else
> > use_bufpoi = 0;
> >
> > --
> > 2.17.1
> >

2019-09-30 16:35:05

by Kamal Dasu

[permalink] [raw]
Subject: Re: [PATCH 2/2] mtd: rawnand: use bounce buffer when vmalloced data buf detected

On Mon, Sep 30, 2019 at 12:25 PM Boris Brezillon
<[email protected]> wrote:
>
> On Mon, 30 Sep 2019 12:01:28 -0400
> Kamal Dasu <[email protected]> wrote:
>
> > Does anyone have any comments on this patch ?.
> >
> > Kamal
> >
> > On Fri, Sep 6, 2019 at 3:49 PM Kamal Dasu <[email protected]> wrote:
> > >
> > > For controller drivers that use DMA and set NAND_USE_BOUNCE_BUFFER
> > > option use data buffers that are not vmalloced, aligned and have
> > > valid virtual address to be able to do DMA transfers. This change
> > > adds additional check and makes use of data buffer allocated
> > > in nand_base driver when it is passed a vmalloced data buffer for
> > > DMA transfers.
> > >
> > > Signed-off-by: Kamal Dasu <[email protected]>
> > > ---
> > > drivers/mtd/nand/raw/nand_base.c | 14 ++++++++------
> > > 1 file changed, 8 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
> > > index 91f046d4d452..46f6965a896a 100644
> > > --- a/drivers/mtd/nand/raw/nand_base.c
> > > +++ b/drivers/mtd/nand/raw/nand_base.c
> > > @@ -45,6 +45,12 @@
> > >
> > > #include "internals.h"
> > >
> > > +static int nand_need_bounce_buf(const void *buf, struct nand_chip *chip)
> > > +{
> > > + return !virt_addr_valid(buf) || is_vmalloc_addr(buf) ||
>
> I thought virt_addr_valid() was implying !is_vmalloc_addr(), are you
> sure you need that test, and can you tell me on which arch(es) this is
> needed.
>

This has been observed on MIPS4K and MIPS5K architectures. There is a
check on the controller driver to use pio in such cases.

> > > + !IS_ALIGNED((unsigned long)buf, chip->buf_align);
> > > +}
> > > +
> > > /* Define default oob placement schemes for large and small page devices */
> > > static int nand_ooblayout_ecc_sp(struct mtd_info *mtd, int section,
> > > struct mtd_oob_region *oobregion)
> > > @@ -3183,9 +3189,7 @@ static int nand_do_read_ops(struct nand_chip *chip, loff_t from,
> > > if (!aligned)
> > > use_bufpoi = 1;
> > > else if (chip->options & NAND_USE_BOUNCE_BUFFER)
> > > - use_bufpoi = !virt_addr_valid(buf) ||
> > > - !IS_ALIGNED((unsigned long)buf,
> > > - chip->buf_align);
> > > + use_bufpoi = nand_need_bounce_buf(buf, chip);
> > > else
> > > use_bufpoi = 0;
> > >
> > > @@ -4009,9 +4013,7 @@ static int nand_do_write_ops(struct nand_chip *chip, loff_t to,
> > > if (part_pagewr)
> > > use_bufpoi = 1;
> > > else if (chip->options & NAND_USE_BOUNCE_BUFFER)
> > > - use_bufpoi = !virt_addr_valid(buf) ||
> > > - !IS_ALIGNED((unsigned long)buf,
> > > - chip->buf_align);
> > > + use_bufpoi = nand_need_bounce_buf(buf, chip);
> > > else
> > > use_bufpoi = 0;
> > >
> > > --
> > > 2.17.1
> > >
>

2019-09-30 20:57:18

by Richard Weinberger

[permalink] [raw]
Subject: Re: [PATCH 2/2] mtd: rawnand: use bounce buffer when vmalloced data buf detected

----- Ursprüngliche Mail -----
> Von: "Kamal Dasu" <[email protected]>
> This has been observed on MIPS4K and MIPS5K architectures. There is a
> check on the controller driver to use pio in such cases.

I fear your kernel misses commit:
074a1e1167af ("MIPS: Bounds check virt_addr_valid")

Thanks,
//richard

2019-10-01 17:08:24

by Kamal Dasu

[permalink] [raw]
Subject: Re: [PATCH 2/2] mtd: rawnand: use bounce buffer when vmalloced data buf detected

I was testing with UBI/UBIFS where vmalloced data buffers can be
passed to the nand_base and then o the controller driver. Probably
applies to older kernel 4.1.

Can the Patch1/2 be accepted or you want me to send it separately by
removing the nand_base changes ?.

Kamal

Kamal

On Mon, Sep 30, 2019 at 3:39 PM Richard Weinberger <[email protected]> wrote:
>
> ----- Ursprüngliche Mail -----
> > Von: "Kamal Dasu" <[email protected]>
> > This has been observed on MIPS4K and MIPS5K architectures. There is a
> > check on the controller driver to use pio in such cases.
>
> I fear your kernel misses commit:
> 074a1e1167af ("MIPS: Bounds check virt_addr_valid")
>
> Thanks,
> //richard