From: qianfan Zhao qianfanguijin@163.com
CHUNK_TYPE_RAW buffer is not aligned, and flash sparse images by fastboot will report "Misaligned operation" if DCACHE is enabled.
Flashing Sparse Image CACHE: Misaligned operation at range [84000028, 84001028] CACHE: Misaligned operation at range [84001034, 84002034] CACHE: Misaligned operation at range [8401104c, 8401304c]
Fix it
Signed-off-by: qianfan Zhao qianfanguijin@163.com --- lib/image-sparse.c | 46 +++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 45 insertions(+), 1 deletion(-)
diff --git a/lib/image-sparse.c b/lib/image-sparse.c index d80fdbbf58..1c621cd685 100644 --- a/lib/image-sparse.c +++ b/lib/image-sparse.c @@ -49,6 +49,48 @@
static void default_log(const char *ignored, char *response) {}
+static lbaint_t write_sparse_chunk_raw(struct sparse_storage *info, + lbaint_t blk, lbaint_t blkcnt, + void *data, + char *response) +{ +#if !CONFIG_IS_ENABLED(SYS_DCACHE_OFF) + lbaint_t n, blks = 0, aligned_buf_blks = 100; + uint32_t *aligned_buf = NULL; + + while (blkcnt > 0) { + aligned_buf = (uint32_t *) + memalign(ARCH_DMA_MINALIGN, + ROUNDUP( + info->blksz * aligned_buf_blks, + ARCH_DMA_MINALIGN)); + if (!aligned_buf) { + info->mssg("Malloc failed for: CHUNK_TYPE_RAW", + response); + return -1; + } + + n = min(aligned_buf_blks, blkcnt); + memcpy(aligned_buf, data, n * info->blksz); + + if (info->write(info, blk + blks, n, aligned_buf) != n) { + free(aligned_buf); + return n + blks; + } + + free(aligned_buf); + + data += n * info->blksz; + blkcnt -= n; + blks += n; + } + + return blks; +#else + return info->write(info, blk, blkcnt, data); +#endif +} + int write_sparse_image(struct sparse_storage *info, const char *part_name, void *data, char *response) { @@ -152,7 +194,9 @@ int write_sparse_image(struct sparse_storage *info, return -1; }
- blks = info->write(info, blk, blkcnt, data); + blks = write_sparse_chunk_raw(info, blk, blkcnt, + data, response); + /* blks might be > blkcnt (eg. NAND bad-blocks) */ if (blks < blkcnt) { printf("%s: %s" LBAFU " [" LBAFU "]\n",
Hi quanfan,
Thanks for the patch. I had something similar in mind.
On 11/5/21 1:46 AM, qianfanguijin@qq.com wrote:
From: qianfan Zhao qianfanguijin@163.com
CHUNK_TYPE_RAW buffer is not aligned, and flash sparse images by fastboot will report "Misaligned operation" if DCACHE is enabled.
Flashing Sparse Image CACHE: Misaligned operation at range [84000028, 84001028] CACHE: Misaligned operation at range [84001034, 84002034] CACHE: Misaligned operation at range [8401104c, 8401304c]
Fix it
Signed-off-by: qianfan Zhao qianfanguijin@163.com
lib/image-sparse.c | 46 +++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 45 insertions(+), 1 deletion(-)
diff --git a/lib/image-sparse.c b/lib/image-sparse.c index d80fdbbf58..1c621cd685 100644 --- a/lib/image-sparse.c +++ b/lib/image-sparse.c @@ -49,6 +49,48 @@
static void default_log(const char *ignored, char *response) {}
+static lbaint_t write_sparse_chunk_raw(struct sparse_storage *info,
lbaint_t blk, lbaint_t blkcnt,void *data,char *response)+{ +#if !CONFIG_IS_ENABLED(SYS_DCACHE_OFF)
Please rewrite this like
if (CONFIG_IS_ENABLED(SYS_DCACHE_OFF) return info->write(info, blk, blkcnt, data);
- lbaint_t n, blks = 0, aligned_buf_blks = 100;
Can we try allocating info->blksz * blkcnt up front?
- uint32_t *aligned_buf = NULL;
- while (blkcnt > 0) {
aligned_buf = (uint32_t *)memalign(ARCH_DMA_MINALIGN,ROUNDUP(info->blksz * aligned_buf_blks,ARCH_DMA_MINALIGN));
I don't think we need this ROUNDUP. info->blksz is the block size of the underlying storage, which is at least 512. And void pointers don't need to be casted.
if (!aligned_buf) {info->mssg("Malloc failed for: CHUNK_TYPE_RAW",response);return -1;
return -ENOMEM;
Yes, the rest of the function returns -1, but there is no reason to perpetuate that.
}n = min(aligned_buf_blks, blkcnt);memcpy(aligned_buf, data, n * info->blksz);if (info->write(info, blk + blks, n, aligned_buf) != n) {free(aligned_buf);
Can we reuse the buffer instead of allocating/freeing it every loop?
return n + blks;}free(aligned_buf);data += n * info->blksz;blkcnt -= n;blks += n;- }
- return blks;
+#else
- return info->write(info, blk, blkcnt, data);
+#endif +}
- int write_sparse_image(struct sparse_storage *info, const char *part_name, void *data, char *response) {
@@ -152,7 +194,9 @@ int write_sparse_image(struct sparse_storage *info, return -1; }
blks = info->write(info, blk, blkcnt, data);
blks = write_sparse_chunk_raw(info, blk, blkcnt,data, response);/* blks might be > blkcnt (eg. NAND bad-blocks) */ if (blks < blkcnt) { printf("%s: %s" LBAFU " [" LBAFU "]\n",
You also need to fix the error handling here. Otherwise you will get tuings like "Write failed, block #4294967295" on out of memory.
--Sean
在 2021/11/5 23:06, Sean Anderson 写道:
Hi quanfan,
Thanks for the patch. I had something similar in mind.
On 11/5/21 1:46 AM, qianfanguijin@qq.com wrote:
From: qianfan Zhao qianfanguijin@163.com
CHUNK_TYPE_RAW buffer is not aligned, and flash sparse images by fastboot will report "Misaligned operation" if DCACHE is enabled.
Flashing Sparse Image CACHE: Misaligned operation at range [84000028, 84001028] CACHE: Misaligned operation at range [84001034, 84002034] CACHE: Misaligned operation at range [8401104c, 8401304c]
Fix it
Signed-off-by: qianfan Zhao qianfanguijin@163.com
lib/image-sparse.c | 46 +++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 45 insertions(+), 1 deletion(-)
diff --git a/lib/image-sparse.c b/lib/image-sparse.c index d80fdbbf58..1c621cd685 100644 --- a/lib/image-sparse.c +++ b/lib/image-sparse.c @@ -49,6 +49,48 @@
static void default_log(const char *ignored, char *response) {}
+static lbaint_t write_sparse_chunk_raw(struct sparse_storage *info, + lbaint_t blk, lbaint_t blkcnt, + void *data, + char *response) +{ +#if !CONFIG_IS_ENABLED(SYS_DCACHE_OFF)
Please rewrite this like
if (CONFIG_IS_ENABLED(SYS_DCACHE_OFF) return info->write(info, blk, blkcnt, data);
+ lbaint_t n, blks = 0, aligned_buf_blks = 100;
Can we try allocating info->blksz * blkcnt up front?
+ uint32_t *aligned_buf = NULL;
+ while (blkcnt > 0) { + aligned_buf = (uint32_t *) + memalign(ARCH_DMA_MINALIGN, + ROUNDUP( + info->blksz * aligned_buf_blks, + ARCH_DMA_MINALIGN));
I don't think we need this ROUNDUP. info->blksz is the block size of the underlying storage, which is at least 512. And void pointers don't need to be casted.
+ if (!aligned_buf) { + info->mssg("Malloc failed for: CHUNK_TYPE_RAW", + response); + return -1;
return -ENOMEM;
Yes, the rest of the function returns -1, but there is no reason to perpetuate that.
+ }
+ n = min(aligned_buf_blks, blkcnt); + memcpy(aligned_buf, data, n * info->blksz);
+ if (info->write(info, blk + blks, n, aligned_buf) != n) { + free(aligned_buf);
Can we reuse the buffer instead of allocating/freeing it every loop?
+ return n + blks; + }
+ free(aligned_buf);
+ data += n * info->blksz; + blkcnt -= n; + blks += n; + }
+ return blks; +#else + return info->write(info, blk, blkcnt, data); +#endif +}
int write_sparse_image(struct sparse_storage *info, const char *part_name, void *data, char *response) { @@ -152,7 +194,9 @@ int write_sparse_image(struct sparse_storage *info, return -1; }
- blks = info->write(info, blk, blkcnt, data); + blks = write_sparse_chunk_raw(info, blk, blkcnt, + data, response);
/* blks might be > blkcnt (eg. NAND bad-blocks) */ if (blks < blkcnt) { printf("%s: %s" LBAFU " [" LBAFU "]\n",
You also need to fix the error handling here. Otherwise you will get tuings like "Write failed, block #4294967295" on out of memory.
thanks for your's review. I had changed based on yours guide and has a question about this error message.
What is your's test platform, nand flash? I had tested on emmc platform and no such messages.
--Sean
On 11/7/21 9:12 PM, qianfan wrote:
在 2021/11/5 23:06, Sean Anderson 写道:
Hi quanfan,
Thanks for the patch. I had something similar in mind.
On 11/5/21 1:46 AM, qianfanguijin@qq.com wrote:
From: qianfan Zhao qianfanguijin@163.com
CHUNK_TYPE_RAW buffer is not aligned, and flash sparse images by fastboot will report "Misaligned operation" if DCACHE is enabled.
Flashing Sparse Image CACHE: Misaligned operation at range [84000028, 84001028] CACHE: Misaligned operation at range [84001034, 84002034] CACHE: Misaligned operation at range [8401104c, 8401304c]
Fix it
Signed-off-by: qianfan Zhao qianfanguijin@163.com
lib/image-sparse.c | 46 +++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 45 insertions(+), 1 deletion(-)
diff --git a/lib/image-sparse.c b/lib/image-sparse.c index d80fdbbf58..1c621cd685 100644 --- a/lib/image-sparse.c +++ b/lib/image-sparse.c @@ -49,6 +49,48 @@
static void default_log(const char *ignored, char *response) {}
+static lbaint_t write_sparse_chunk_raw(struct sparse_storage *info, + lbaint_t blk, lbaint_t blkcnt, + void *data, + char *response) +{ +#if !CONFIG_IS_ENABLED(SYS_DCACHE_OFF)
Please rewrite this like
if (CONFIG_IS_ENABLED(SYS_DCACHE_OFF) return info->write(info, blk, blkcnt, data);
+ lbaint_t n, blks = 0, aligned_buf_blks = 100;
Can we try allocating info->blksz * blkcnt up front?
+ uint32_t *aligned_buf = NULL;
+ while (blkcnt > 0) { + aligned_buf = (uint32_t *) + memalign(ARCH_DMA_MINALIGN, + ROUNDUP( + info->blksz * aligned_buf_blks, + ARCH_DMA_MINALIGN));
I don't think we need this ROUNDUP. info->blksz is the block size of the underlying storage, which is at least 512. And void pointers don't need to be casted.
+ if (!aligned_buf) { + info->mssg("Malloc failed for: CHUNK_TYPE_RAW", + response); + return -1;
return -ENOMEM;
Yes, the rest of the function returns -1, but there is no reason to perpetuate that.
+ }
+ n = min(aligned_buf_blks, blkcnt); + memcpy(aligned_buf, data, n * info->blksz);
+ if (info->write(info, blk + blks, n, aligned_buf) != n) { + free(aligned_buf);
Can we reuse the buffer instead of allocating/freeing it every loop?
+ return n + blks; + }
+ free(aligned_buf);
+ data += n * info->blksz; + blkcnt -= n; + blks += n; + }
+ return blks; +#else + return info->write(info, blk, blkcnt, data); +#endif +}
int write_sparse_image(struct sparse_storage *info, const char *part_name, void *data, char *response) { @@ -152,7 +194,9 @@ int write_sparse_image(struct sparse_storage *info, return -1; }
- blks = info->write(info, blk, blkcnt, data); + blks = write_sparse_chunk_raw(info, blk, blkcnt, + data, response);
/* blks might be > blkcnt (eg. NAND bad-blocks) */ if (blks < blkcnt) { printf("%s: %s" LBAFU " [" LBAFU "]\n",
You also need to fix the error handling here. Otherwise you will get tuings like "Write failed, block #4294967295" on out of memory.
thanks for your's review. I had changed based on yours guide and has a question about this error message.
What is your's test platform, nand flash? I had tested on emmc platform and no such messages.
write_sparse_chunk_raw has two error conditions: blks < blkcnt, and blks < 0. The latter case will be handled incorrectly.
--Sean
participants (3)
-
qianfan -
qianfanguijin＠qq.com
-
Sean Anderson