Re: [PATCH v4] lib: sparse: Make CHUNK_TYPE_RAW buffer aligned
On Tue, Nov 16, 2021 at 09:35:38AM +0800, qianfangui...@qq.com wrote: > From: qianfan Zhao > > 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 [8428, 84001028] > CACHE: Misaligned operation at range [84001034, 84002034] > CACHE: Misaligned operation at range [8401104c, 8401304c] > > Fix it > > Signed-off-by: qianfan Zhao > Reviewed-by: Sean Anderson Applied to u-boot/master, thanks! -- Tom signature.asc Description: PGP signature
Re: [PATCH v4] lib: sparse: Make CHUNK_TYPE_RAW buffer aligned
On 11/17/21 3:41 AM, qianfan wrote:
在 2021/11/16 23:05, Sean Anderson 写道:
On 11/15/21 8:35 PM, qianfangui...@qq.com wrote:
From: qianfan Zhao
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 [8428, 84001028]
CACHE: Misaligned operation at range [84001034, 84002034]
CACHE: Misaligned operation at range [8401104c, 8401304c]
Fix it
Signed-off-by: qianfan Zhao
---
lib/image-sparse.c | 69 --
1 file changed, 61 insertions(+), 8 deletions(-)
diff --git a/lib/image-sparse.c b/lib/image-sparse.c
index d80fdbbf58..5ec0f94ab3 100644
--- a/lib/image-sparse.c
+++ b/lib/image-sparse.c
@@ -46,9 +46,66 @@
#include
#include
+#include
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)
+{
+ lbaint_t n = blkcnt, write_blks, blks = 0, aligned_buf_blks = 100;
+ uint32_t *aligned_buf = NULL;
+
+ if (CONFIG_IS_ENABLED(SYS_DCACHE_OFF)) {
+ write_blks = info->write(info, blk, n, data);
+ if (write_blks < n)
+ goto write_fail;
+
+ return write_blks;
+ }
+
+ aligned_buf = memalign(ARCH_DMA_MINALIGN, info->blksz * aligned_buf_blks);
+ if (!aligned_buf) {
+ info->mssg("Malloc failed for: CHUNK_TYPE_RAW", response);
+ return -ENOMEM;
+ }
+
+ while (blkcnt > 0) {
+ n = min(aligned_buf_blks, blkcnt);
+ memcpy(aligned_buf, data, n * info->blksz);
+
+ /* write_blks might be > n due to NAND bad-blocks */
nit: <
This comment is misleading here, but it is actually correct. It is correct to
write more that the specified number when nand flash bad blocks occur, so
(write_blks >= n) is write successful. maybe rewrite the judge conditions like
this is better?
Ah, you are right.
if (!(write_blks >= n))
+ write_blks = info->write(info, blk + blks, n, aligned_buf);
+ if (write_blks < n) {
+ free(aligned_buf);
+ goto write_fail;
+ }
+
+ blks += write_blks;
+ data += n * info->blksz;
+ blkcnt -= n;
+ }
+
+ free(aligned_buf);
+ return blks;
+
+write_fail:
I think this label can be lower, but it does not affect correctness.
sorry, could you please explain this clear, I can't understand.
+ if (IS_ERR_VALUE(write_blks)) {
+ printf("%s: Write failed, block #" LBAFU " [" LBAFU "] (%lld)\n",
+ __func__, blk + blks, n, (long long)write_blks);
+ info->mssg("flash write failure", response);
+ return write_blks;
+ }
The above label could be placed here, since you check the value of write_blks
before jumping.
--Sean
+ /* write_blks < n */
+ printf("%s: Write failed, block #" LBAFU " [" LBAFU "]\n",
+ __func__, blk + blks, n);
+ info->mssg("flash write failure(incomplete)", response);
+ return -1;
-EIO?
It's OK
+}
+
int write_sparse_image(struct sparse_storage *info,
const char *part_name, void *data, char *response)
{
@@ -152,15 +209,11 @@ int write_sparse_image(struct sparse_storage *info,
return -1;
}
- blks = info->write(info, blk, blkcnt, data);
- /* blks might be > blkcnt (eg. NAND bad-blocks) */
- if (blks < blkcnt) {
- printf("%s: %s" LBAFU " [" LBAFU "]\n",
- __func__, "Write failed, block #",
- blk, blks);
- info->mssg("flash write failure", response);
+ blks = write_sparse_chunk_raw(info, blk, blkcnt,
+ data, response);
+ if (blks < 0)
return -1;
ditto
It's OK
- }
+
blk += blks;
bytes_written += ((u64)blkcnt) * info->blksz;
total_blocks += chunk_header->chunk_sz;
Reviewed-by: Sean Anderson
Re: [PATCH v4] lib: sparse: Make CHUNK_TYPE_RAW buffer aligned
On 11/15/21 8:35 PM, qianfangui...@qq.com wrote:
From: qianfan Zhao
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 [8428, 84001028]
CACHE: Misaligned operation at range [84001034, 84002034]
CACHE: Misaligned operation at range [8401104c, 8401304c]
Fix it
Signed-off-by: qianfan Zhao
---
lib/image-sparse.c | 69 --
1 file changed, 61 insertions(+), 8 deletions(-)
diff --git a/lib/image-sparse.c b/lib/image-sparse.c
index d80fdbbf58..5ec0f94ab3 100644
--- a/lib/image-sparse.c
+++ b/lib/image-sparse.c
@@ -46,9 +46,66 @@
#include
#include
+#include
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)
+{
+ lbaint_t n = blkcnt, write_blks, blks = 0, aligned_buf_blks = 100;
+ uint32_t *aligned_buf = NULL;
+
+ if (CONFIG_IS_ENABLED(SYS_DCACHE_OFF)) {
+ write_blks = info->write(info, blk, n, data);
+ if (write_blks < n)
+ goto write_fail;
+
+ return write_blks;
+ }
+
+ aligned_buf = memalign(ARCH_DMA_MINALIGN, info->blksz *
aligned_buf_blks);
+ if (!aligned_buf) {
+ info->mssg("Malloc failed for: CHUNK_TYPE_RAW", response);
+ return -ENOMEM;
+ }
+
+ while (blkcnt > 0) {
+ n = min(aligned_buf_blks, blkcnt);
+ memcpy(aligned_buf, data, n * info->blksz);
+
+ /* write_blks might be > n due to NAND bad-blocks */
nit: <
+ write_blks = info->write(info, blk + blks, n, aligned_buf);
+ if (write_blks < n) {
+ free(aligned_buf);
+ goto write_fail;
+ }
+
+ blks += write_blks;
+ data += n * info->blksz;
+ blkcnt -= n;
+ }
+
+ free(aligned_buf);
+ return blks;
+
+write_fail:
I think this label can be lower, but it does not affect correctness.
+ if (IS_ERR_VALUE(write_blks)) {
+ printf("%s: Write failed, block #" LBAFU " [" LBAFU "]
(%lld)\n",
+ __func__, blk + blks, n, (long long)write_blks);
+ info->mssg("flash write failure", response);
+ return write_blks;
+ }
+
+ /* write_blks < n */
+ printf("%s: Write failed, block #" LBAFU " [" LBAFU "]\n",
+ __func__, blk + blks, n);
+ info->mssg("flash write failure(incomplete)", response);
+ return -1;
-EIO?
+}
+
int write_sparse_image(struct sparse_storage *info,
const char *part_name, void *data, char *response)
{
@@ -152,15 +209,11 @@ int write_sparse_image(struct sparse_storage *info,
return -1;
}
- blks = info->write(info, blk, blkcnt, data);
- /* blks might be > blkcnt (eg. NAND bad-blocks) */
- if (blks < blkcnt) {
- printf("%s: %s" LBAFU " [" LBAFU "]\n",
- __func__, "Write failed, block #",
- blk, blks);
- info->mssg("flash write failure", response);
+ blks = write_sparse_chunk_raw(info, blk, blkcnt,
+ data, response);
+ if (blks < 0)
return -1;
ditto
- }
+
blk += blks;
bytes_written += ((u64)blkcnt) * info->blksz;
total_blocks += chunk_header->chunk_sz;
Reviewed-by: Sean Anderson
[PATCH v4] lib: sparse: Make CHUNK_TYPE_RAW buffer aligned
From: qianfan Zhao
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 [8428, 84001028]
CACHE: Misaligned operation at range [84001034, 84002034]
CACHE: Misaligned operation at range [8401104c, 8401304c]
Fix it
Signed-off-by: qianfan Zhao
---
lib/image-sparse.c | 69 --
1 file changed, 61 insertions(+), 8 deletions(-)
diff --git a/lib/image-sparse.c b/lib/image-sparse.c
index d80fdbbf58..5ec0f94ab3 100644
--- a/lib/image-sparse.c
+++ b/lib/image-sparse.c
@@ -46,9 +46,66 @@
#include
#include
+#include
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)
+{
+ lbaint_t n = blkcnt, write_blks, blks = 0, aligned_buf_blks = 100;
+ uint32_t *aligned_buf = NULL;
+
+ if (CONFIG_IS_ENABLED(SYS_DCACHE_OFF)) {
+ write_blks = info->write(info, blk, n, data);
+ if (write_blks < n)
+ goto write_fail;
+
+ return write_blks;
+ }
+
+ aligned_buf = memalign(ARCH_DMA_MINALIGN, info->blksz *
aligned_buf_blks);
+ if (!aligned_buf) {
+ info->mssg("Malloc failed for: CHUNK_TYPE_RAW", response);
+ return -ENOMEM;
+ }
+
+ while (blkcnt > 0) {
+ n = min(aligned_buf_blks, blkcnt);
+ memcpy(aligned_buf, data, n * info->blksz);
+
+ /* write_blks might be > n due to NAND bad-blocks */
+ write_blks = info->write(info, blk + blks, n, aligned_buf);
+ if (write_blks < n) {
+ free(aligned_buf);
+ goto write_fail;
+ }
+
+ blks += write_blks;
+ data += n * info->blksz;
+ blkcnt -= n;
+ }
+
+ free(aligned_buf);
+ return blks;
+
+write_fail:
+ if (IS_ERR_VALUE(write_blks)) {
+ printf("%s: Write failed, block #" LBAFU " [" LBAFU "]
(%lld)\n",
+ __func__, blk + blks, n, (long long)write_blks);
+ info->mssg("flash write failure", response);
+ return write_blks;
+ }
+
+ /* write_blks < n */
+ printf("%s: Write failed, block #" LBAFU " [" LBAFU "]\n",
+ __func__, blk + blks, n);
+ info->mssg("flash write failure(incomplete)", response);
+ return -1;
+}
+
int write_sparse_image(struct sparse_storage *info,
const char *part_name, void *data, char *response)
{
@@ -152,15 +209,11 @@ int write_sparse_image(struct sparse_storage *info,
return -1;
}
- blks = info->write(info, blk, blkcnt, data);
- /* blks might be > blkcnt (eg. NAND bad-blocks) */
- if (blks < blkcnt) {
- printf("%s: %s" LBAFU " [" LBAFU "]\n",
- __func__, "Write failed, block #",
- blk, blks);
- info->mssg("flash write failure", response);
+ blks = write_sparse_chunk_raw(info, blk, blkcnt,
+ data, response);
+ if (blks < 0)
return -1;
- }
+
blk += blks;
bytes_written += ((u64)blkcnt) * info->blksz;
total_blocks += chunk_header->chunk_sz;
--
2.17.1
