flash/nor/core: Fix chunk size calculation in default_flash_mem_blank_check
[openocd.git] / src / flash / nor / core.c
index 707dcff181a21e0bfa6a48648cf3480e253a7c4b..c541afcd06cccce6243f0e72438d793fb6385ad6 100644 (file)
@@ -4,6 +4,7 @@
  *   Copyright (C) 2008 by Spencer Oliver <spen@spen-soft.co.uk>           *
  *   Copyright (C) 2009 Zachary T Welch <zw@superlucidity.net>             *
  *   Copyright (C) 2010 by Antonio Borneo <borneo.antonio@gmail.com>       *
+ *   Copyright (C) 2017-2018 Tomas Vanek <vanekt@fbl.cz>                   *
  *                                                                         *
  *   This program is free software; you can redistribute it and/or modify  *
  *   it under the terms of the GNU General Public License as published by  *
@@ -67,6 +68,11 @@ int flash_driver_protect(struct flash_bank *bank, int set, int first, int last)
        /* force "set" to 0/1 */
        set = !!set;
 
+       if (bank->driver->protect == NULL) {
+               LOG_ERROR("Flash protection is not supported.");
+               return ERROR_FLASH_OPER_UNSUPPORTED;
+       }
+
        /* DANGER!
         *
         * We must not use any cached information about protection state!!!!
@@ -187,9 +193,17 @@ void flash_free_all_banks(void)
                else
                        LOG_WARNING("Flash driver of %s does not support free_driver_priv()", bank->name);
 
+               /* For 'virtual' flash driver bank->sectors and bank->prot_blocks pointers are copied from
+                * master flash_bank structure. They point to memory locations allocated by master flash driver
+                * so master driver is responsible for releasing them.
+                * Avoid UB caused by double-free memory corruption if flash bank is 'virtual'. */
+
+               if (strcmp(bank->driver->name, "virtual") != 0) {
+                       free(bank->sectors);
+                       free(bank->prot_blocks);
+               }
+
                free(bank->name);
-               free(bank->sectors);
-               free(bank->prot_blocks);
                free(bank);
                bank = next;
        }
@@ -308,8 +322,8 @@ static int default_flash_mem_blank_check(struct flash_bank *bank)
                for (j = 0; j < bank->sectors[i].size; j += buffer_size) {
                        uint32_t chunk;
                        chunk = buffer_size;
-                       if (chunk > (j - bank->sectors[i].size))
-                               chunk = (j - bank->sectors[i].size);
+                       if (chunk > (bank->sectors[i].size - j))
+                               chunk = (bank->sectors[i].size - j);
 
                        retval = target_read_memory(target,
                                        bank->base + bank->sectors[i].offset + j,
@@ -600,6 +614,87 @@ static int compare_section(const void *a, const void *b)
                return -1;
 }
 
+/**
+ * Get aligned start address of a flash write region
+ */
+target_addr_t flash_write_align_start(struct flash_bank *bank, target_addr_t addr)
+{
+       if (addr < bank->base || addr >= bank->base + bank->size
+                       || bank->write_start_alignment <= 1)
+               return addr;
+
+       if (bank->write_start_alignment == FLASH_WRITE_ALIGN_SECTOR) {
+               uint32_t offset = addr - bank->base;
+               uint32_t aligned = 0;
+               int sect;
+               for (sect = 0; sect < bank->num_sectors; sect++) {
+                       if (bank->sectors[sect].offset > offset)
+                               break;
+
+                       aligned = bank->sectors[sect].offset;
+               }
+               return bank->base + aligned;
+       }
+
+       return addr & ~(bank->write_start_alignment - 1);
+}
+
+/**
+ * Get aligned end address of a flash write region
+ */
+target_addr_t flash_write_align_end(struct flash_bank *bank, target_addr_t addr)
+{
+       if (addr < bank->base || addr >= bank->base + bank->size
+                       || bank->write_end_alignment <= 1)
+               return addr;
+
+       if (bank->write_end_alignment == FLASH_WRITE_ALIGN_SECTOR) {
+               uint32_t offset = addr - bank->base;
+               uint32_t aligned = 0;
+               int sect;
+               for (sect = 0; sect < bank->num_sectors; sect++) {
+                       aligned = bank->sectors[sect].offset + bank->sectors[sect].size - 1;
+                       if (aligned >= offset)
+                               break;
+               }
+               return bank->base + aligned;
+       }
+
+       return addr | (bank->write_end_alignment - 1);
+}
+
+/**
+ * Check if gap between sections is bigger than minimum required to discontinue flash write
+ */
+static bool flash_write_check_gap(struct flash_bank *bank,
+                               target_addr_t addr1, target_addr_t addr2)
+{
+       if (bank->minimal_write_gap == FLASH_WRITE_CONTINUOUS
+                       || addr1 < bank->base || addr1 >= bank->base + bank->size
+                       || addr2 < bank->base || addr2 >= bank->base + bank->size)
+               return false;
+
+       if (bank->minimal_write_gap == FLASH_WRITE_GAP_SECTOR) {
+               int sect;
+               uint32_t offset1 = addr1 - bank->base;
+               /* find the sector following the one containing addr1 */
+               for (sect = 0; sect < bank->num_sectors; sect++) {
+                       if (bank->sectors[sect].offset > offset1)
+                               break;
+               }
+               if (sect >= bank->num_sectors)
+                       return false;
+
+               uint32_t offset2 = addr2 - bank->base;
+               return bank->sectors[sect].offset + bank->sectors[sect].size <= offset2;
+       }
+
+       target_addr_t aligned1 = flash_write_align_end(bank, addr1);
+       target_addr_t aligned2 = flash_write_align_start(bank, addr2);
+       return aligned1 + bank->minimal_write_gap < aligned2;
+}
+
+
 int flash_write_unlock(struct target *target, struct image *image,
        uint32_t *written, int erase, bool unlock)
 {
@@ -639,7 +734,7 @@ int flash_write_unlock(struct target *target, struct image *image,
 
        /* loop until we reach end of the image */
        while (section < image->num_sections) {
-               uint32_t buffer_size;
+               uint32_t buffer_idx;
                uint8_t *buffer;
                int section_last;
                target_addr_t run_address = sections[section]->base_address + section_offset;
@@ -676,43 +771,37 @@ int flash_write_unlock(struct target *target, struct image *image,
                                break;
                        }
 
-                       /* FIXME This needlessly touches sectors BETWEEN the
-                        * sections it's writing.  Without auto erase, it just
-                        * writes ones.  That WILL INVALIDATE data in cases
-                        * like Stellaris Tempest chips, corrupting internal
-                        * ECC codes; and at least FreeScale suggests issues
-                        * with that approach (in HC11 documentation).
-                        *
-                        * With auto erase enabled, data in those sectors will
-                        * be needlessly destroyed; and some of the limited
-                        * number of flash erase cycles will be wasted...
-                        *
-                        * In both cases, the extra writes slow things down.
-                        */
-
                        /* if we have multiple sections within our image,
                         * flash programming could fail due to alignment issues
                         * attempt to rebuild a consecutive buffer for the flash loader */
                        target_addr_t run_next_addr = run_address + run_size;
-                       if (sections[section_last + 1]->base_address < run_next_addr) {
+                       target_addr_t next_section_base = sections[section_last + 1]->base_address;
+                       if (next_section_base < run_next_addr) {
                                LOG_ERROR("Section at " TARGET_ADDR_FMT
                                        " overlaps section ending at " TARGET_ADDR_FMT,
-                                       sections[section_last + 1]->base_address,
-                                       run_next_addr);
+                                       next_section_base, run_next_addr);
                                LOG_ERROR("Flash write aborted.");
                                retval = ERROR_FAIL;
                                goto done;
                        }
 
-                       pad_bytes = sections[section_last + 1]->base_address - run_next_addr;
+                       pad_bytes = next_section_base - run_next_addr;
+                       if (pad_bytes) {
+                               if (flash_write_check_gap(c, run_next_addr - 1, next_section_base)) {
+                                       LOG_INFO("Flash write discontinued at " TARGET_ADDR_FMT
+                                               ", next section at " TARGET_ADDR_FMT,
+                                               run_next_addr, next_section_base);
+                                       break;
+                               }
+                       }
+                       if (pad_bytes > 0)
+                               LOG_INFO("Padding image section %d at " TARGET_ADDR_FMT
+                                       " with %d bytes",
+                                       section_last, run_next_addr, pad_bytes);
+
                        padding[section_last] = pad_bytes;
-                       run_size += sections[++section_last]->size;
                        run_size += pad_bytes;
-
-                       if (pad_bytes > 0)
-                               LOG_INFO("Padding image section %d with %d bytes",
-                                       section_last-1,
-                                       pad_bytes);
+                       run_size += sections[++section_last]->size;
                }
 
                if (run_address + run_size - 1 > c->base + c->size - 1) {
@@ -725,10 +814,38 @@ int flash_write_unlock(struct target *target, struct image *image,
                        assert(run_size > 0);
                }
 
-               /* If we're applying any sector automagic, then pad this
-                * (maybe-combined) segment to the end of its last sector.
-                */
-               if (unlock || erase) {
+               uint32_t padding_at_start = 0;
+               if (c->write_start_alignment || c->write_end_alignment) {
+                       /* align write region according to bank requirements */
+                       target_addr_t aligned_start = flash_write_align_start(c, run_address);
+                       padding_at_start = run_address - aligned_start;
+                       if (padding_at_start > 0) {
+                               LOG_WARNING("Section start address " TARGET_ADDR_FMT
+                                       " breaks the required alignment of flash bank %s",
+                                       run_address, c->name);
+                               LOG_WARNING("Padding %d bytes from " TARGET_ADDR_FMT,
+                                       padding_at_start, aligned_start);
+
+                               run_address -= padding_at_start;
+                               run_size += padding_at_start;
+                       }
+
+                       target_addr_t run_end = run_address + run_size - 1;
+                       target_addr_t aligned_end = flash_write_align_end(c, run_end);
+                       pad_bytes = aligned_end - run_end;
+                       if (pad_bytes > 0) {
+                               LOG_INFO("Padding image section %d at " TARGET_ADDR_FMT
+                                       " with %d bytes (bank write end alignment)",
+                                       section_last, run_end + 1, pad_bytes);
+
+                               padding[section_last] += pad_bytes;
+                               run_size += pad_bytes;
+                       }
+
+               } else if (unlock || erase) {
+                       /* If we're applying any sector automagic, then pad this
+                        * (maybe-combined) segment to the end of its last sector.
+                        */
                        int sector;
                        uint32_t offset_start = run_address - c->base;
                        uint32_t offset_end = offset_start + run_size;
@@ -753,13 +870,17 @@ int flash_write_unlock(struct target *target, struct image *image,
                        retval = ERROR_FAIL;
                        goto done;
                }
-               buffer_size = 0;
+
+               if (padding_at_start)
+                       memset(buffer, c->default_padded_value, padding_at_start);
+
+               buffer_idx = padding_at_start;
 
                /* read sections to the buffer */
-               while (buffer_size < run_size) {
+               while (buffer_idx < run_size) {
                        size_t size_read;
 
-                       size_read = run_size - buffer_size;
+                       size_read = run_size - buffer_idx;
                        if (size_read > sections[section]->size - section_offset)
                                size_read = sections[section]->size - section_offset;
 
@@ -772,23 +893,25 @@ int flash_write_unlock(struct target *target, struct image *image,
                        int t_section_num = diff / sizeof(struct imagesection);
 
                        LOG_DEBUG("image_read_section: section = %d, t_section_num = %d, "
-                                       "section_offset = %d, buffer_size = %d, size_read = %d",
-                               (int)section, (int)t_section_num, (int)section_offset,
-                               (int)buffer_size, (int)size_read);
+                                       "section_offset = %"PRIu32", buffer_idx = %"PRIu32", size_read = %zu",
+                               section, t_section_num, section_offset,
+                               buffer_idx, size_read);
                        retval = image_read_section(image, t_section_num, section_offset,
-                                       size_read, buffer + buffer_size, &size_read);
+                                       size_read, buffer + buffer_idx, &size_read);
                        if (retval != ERROR_OK || size_read == 0) {
                                free(buffer);
                                goto done;
                        }
 
-                       /* see if we need to pad the section */
-                       while (padding[section]--)
-                               (buffer + buffer_size)[size_read++] = c->default_padded_value;
-
-                       buffer_size += size_read;
+                       buffer_idx += size_read;
                        section_offset += size_read;
 
+                       /* see if we need to pad the section */
+                       if (padding[section]) {
+                               memset(buffer + buffer_idx, c->default_padded_value, padding[section]);
+                               buffer_idx += padding[section];
+                       }
+
                        if (section_offset >= sections[section]->size) {
                                section++;
                                section_offset = 0;

Linking to existing account procedure

If you already have an account and want to add another login method you MUST first sign in with your existing account and then change URL to read https://review.openocd.org/login/?link to get to this page again but this time it'll work for linking. Thank you.

SSH host keys fingerprints

1024 SHA256:YKx8b7u5ZWdcbp7/4AeXNaqElP49m6QrwfXaqQGJAOk gerrit-code-review@openocd.zylin.com (DSA)
384 SHA256:jHIbSQa4REvwCFG4cq5LBlBLxmxSqelQPem/EXIrxjk gerrit-code-review@openocd.org (ECDSA)
521 SHA256:UAOPYkU9Fjtcao0Ul/Rrlnj/OsQvt+pgdYSZ4jOYdgs gerrit-code-review@openocd.org (ECDSA)
256 SHA256:A13M5QlnozFOvTllybRZH6vm7iSt0XLxbA48yfc2yfY gerrit-code-review@openocd.org (ECDSA)
256 SHA256:spYMBqEYoAOtK7yZBrcwE8ZpYt6b68Cfh9yEVetvbXg gerrit-code-review@openocd.org (ED25519)
+--[ED25519 256]--+
|=..              |
|+o..   .         |
|*.o   . .        |
|+B . . .         |
|Bo. = o S        |
|Oo.+ + =         |
|oB=.* = . o      |
| =+=.+   + E     |
|. .=o   . o      |
+----[SHA256]-----+
2048 SHA256:0Onrb7/PHjpo6iVZ7xQX2riKN83FJ3KGU0TvI0TaFG4 gerrit-code-review@openocd.zylin.com (RSA)