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 <qianfangui...@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 <qianfangui...@163.com>
---
  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 <asm/cache.h>
    #include <linux/math64.h>
+#include <linux/err.h>
    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 <sean.ander...@seco.com>

Reply via email to