| Message ID | fb7ecceadecacd4498ca27ac2481252bffc4a36e.1436175196.git.marcel.ziswiler@toradex.com |
|---|---|
| State | Superseded |
| Delegated to: | Tom Rini |
| Headers | show |
On 8 July 2015 at 05:58, Marcel Ziswiler <marcel@ziswiler.com> wrote: > > From: Marcel Ziswiler <marcel.ziswiler@toradex.com> > > Various U-Boot adoptions/extensions to MTD/NAND/UBI did not take buffer > alignment into account which led to failures of the following form: > > ERROR: v7_dcache_inval_range - start address is not aligned - 0x1f7f0108 > ERROR: v7_dcache_inval_range - stop address is not aligned - 0x1f7f1108 > > Signed-off-by: Marcel Ziswiler <marcel.ziswiler@toradex.com> > --- > Changes in v2: run it through checkpatch.pl and fix long lines > > common/cmd_ubi.c | 2 +- > drivers/mtd/nand/nand_util.c | 2 +- > fs/ubifs/super.c | 6 ++++-- > fs/ubifs/ubifs.c | 5 +++-- > lib/gzip.c | 2 +- > 5 files changed, 10 insertions(+), 7 deletions(-) Reviewed-by: Simon Glass <sjg@chromium.org>
On Wed, 2015-07-08 at 13:58 +0200, Marcel Ziswiler wrote: > From: Marcel Ziswiler <marcel.ziswiler@toradex.com> > > Various U-Boot adoptions/extensions to MTD/NAND/UBI did not take buffer > alignment into account which led to failures of the following form: > > ERROR: v7_dcache_inval_range - start address is not aligned - 0x1f7f0108 > ERROR: v7_dcache_inval_range - stop address is not aligned - 0x1f7f1108 > > Signed-off-by: Marcel Ziswiler <marcel.ziswiler@toradex.com> > --- > Changes in v2: run it through checkpatch.pl and fix long lines > > common/cmd_ubi.c | 2 +- > drivers/mtd/nand/nand_util.c | 2 +- > fs/ubifs/super.c | 6 ++++-- > fs/ubifs/ubifs.c | 5 +++-- > lib/gzip.c | 2 +- > 5 files changed, 10 insertions(+), 7 deletions(-) As per discussion on v1, this isn't enough to guarantee that the stop address will be aligned. There needs to be a wrapper around memalign() that cache- aligns the size as well. -Scott
Hi Scott On Wed, 2015-07-08 at 18:25 -0500, Scott Wood wrote: > > As per discussion on v1, this isn't enough to guarantee that the stop address > will be aligned. There needs to be a wrapper around memalign() that cache- > aligns the size as well. > > -Scott Sorry, that isn't quite clear to me yet. You're saying I would need to handle that in this patch? Looking through the sources I actually found only one single usage of memalign() which explicitly takes care of this the way you propose: drivers/usb/host/xhci-mem.c: ptr = memalign(cacheline_size, ALIGN(size, cacheline_size)); I'm just wondering about the 107 other places in U-Boot where memalign() is already used the way I do in this patch. Wouldn't there need to be an infrastructure solution done to e.g. memalign() itself? Cheers Marcel
On Thu, 2015-07-09 at 09:47 +0200, Marcel Ziswiler wrote: > Hi Scott > > On Wed, 2015-07-08 at 18:25 -0500, Scott Wood wrote: > > > > As per discussion on v1, this isn't enough to guarantee that the stop > > address > > will be aligned. There needs to be a wrapper around memalign() that > > cache- > > aligns the size as well. > > > > -Scott > > Sorry, that isn't quite clear to me yet. You're saying I would need to > handle that in this patch? > > Looking through the sources I actually found only one single usage of > memalign() which explicitly takes care of this the way you propose: > > drivers/usb/host/xhci-mem.c: ptr = memalign(cacheline_size, ALIGN(size, > cacheline_size)); > > I'm just wondering about the 107 other places in U-Boot where memalign() > is already used the way I do in this patch. I suppose there are enough existing ones that it won't hurt to add more -- it's still an improvement over the current situation -- but it'd be good to fix it properly before it gets forgotten. > Wouldn't there need to be an infrastructure solution done to e.g. > memalign() itself? No, just a wrapper: static inline void *malloc_cache_aligned(size_t size) { return memalign(ARCH_DMA_MINALIGN, ALIGN(size, ARCH_DMA_MINALIGN)); }
diff --git a/common/cmd_ubi.c b/common/cmd_ubi.c index cbc10c5..30a1259 100644 --- a/common/cmd_ubi.c +++ b/common/cmd_ubi.c @@ -363,7 +363,7 @@ int ubi_volume_read(char *volume, char *buf, size_t size) tbuf_size = vol->usable_leb_size; if (size < tbuf_size) tbuf_size = ALIGN(size, ubi->min_io_size); - tbuf = malloc(tbuf_size); + tbuf = memalign(ARCH_DMA_MINALIGN, tbuf_size); if (!tbuf) { printf("NO MEM\n"); return ENOMEM; diff --git a/drivers/mtd/nand/nand_util.c b/drivers/mtd/nand/nand_util.c index ee2c24d..395ba2d 100644 --- a/drivers/mtd/nand/nand_util.c +++ b/drivers/mtd/nand/nand_util.c @@ -839,7 +839,7 @@ int nand_torture(nand_info_t *nand, loff_t offset) patt_count = ARRAY_SIZE(patterns); - buf = malloc(nand->erasesize); + buf = memalign(ARCH_DMA_MINALIGN, nand->erasesize); if (buf == NULL) { puts("Out of memory for erase block buffer\n"); return -ENOMEM; diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c index 10f8fff..daa089b 100644 --- a/fs/ubifs/super.c +++ b/fs/ubifs/super.c @@ -57,7 +57,8 @@ struct inode *iget_locked(struct super_block *sb, unsigned long ino) { struct inode *inode; - inode = (struct inode *)malloc(sizeof(struct ubifs_inode)); + inode = (struct inode *)memalign(ARCH_DMA_MINALIGN, + sizeof(struct ubifs_inode)); if (inode) { inode->i_ino = ino; inode->i_sb = sb; @@ -104,7 +105,8 @@ void iput(struct inode *inode) /* * Allocate and use new inode */ - ino = (struct inode *)malloc(sizeof(struct ubifs_inode)); + ino = (struct inode *)memalign(ARCH_DMA_MINALIGN, + sizeof(struct ubifs_inode)); memcpy(ino, inode, sizeof(struct ubifs_inode)); /* diff --git a/fs/ubifs/ubifs.c b/fs/ubifs/ubifs.c index 6dd6174..a0daf3e 100644 --- a/fs/ubifs/ubifs.c +++ b/fs/ubifs/ubifs.c @@ -108,7 +108,7 @@ static inline struct crypto_comp *crypto_alloc_comp(const char *alg_name, struct crypto_comp *ptr; int i = 0; - ptr = malloc(sizeof(struct crypto_comp)); + ptr = memalign(ARCH_DMA_MINALIGN, sizeof(struct crypto_comp)); while (i < UBIFS_COMPR_TYPES_CNT) { comp = ubifs_compressors[i]; if (!comp) { @@ -723,7 +723,8 @@ static int do_readpage(struct ubifs_info *c, struct inode *inode, * destination area to a multiple of * UBIFS_BLOCK_SIZE. */ - buff = malloc(UBIFS_BLOCK_SIZE); + buff = memalign(ARCH_DMA_MINALIGN, + UBIFS_BLOCK_SIZE); if (!buff) { printf("%s: Error, malloc fails!\n", __func__); diff --git a/lib/gzip.c b/lib/gzip.c index ff37d4f..2a74411 100644 --- a/lib/gzip.c +++ b/lib/gzip.c @@ -25,7 +25,7 @@ static void *zalloc(void *x, unsigned items, unsigned size) size *= items; size = (size + ZALLOC_ALIGNMENT - 1) & ~(ZALLOC_ALIGNMENT - 1); - p = malloc (size); + p = memalign(ARCH_DMA_MINALIGN, size); return (p); }