From a452b83cd9eac584073fa19eb76e133de913ef52 Mon Sep 17 00:00:00 2001 From: Cyril Bur Date: Tue, 26 Jul 2016 13:49:33 +1000 Subject: libflash/blocklevel: Allow double ecc protecting a region Currently the policy for calling ECC protecting a range at the blocklevel layer is that the requested region be completely unprotected otherwise the call will return an error. It turns out that duplicate calls to ffs_init() with true as the last parameter (for the same blocklevel structure) will cause duplicate attempts to ecc_protect() ranges. Change the policy within blocklevel to allow duplicate protecting. In fact the new policy almost guarantees no failure (baring something odd like malloc() failing). It will detect that the range is currently already fully protected and do nothing, detect that part of the range is or is not and extend the existing range or detect that a range fits perfectly between two ranges in which case it will merge the ranges. Also adjust tests to match the new policy. Signed-off-by: Cyril Bur Signed-off-by: Stewart Smith --- libflash/blocklevel.c | 114 ++++++++++++++++++++++++++-------------- libflash/libffs.c | 2 +- libflash/test/test-blocklevel.c | 54 ++++++++++++++----- 3 files changed, 119 insertions(+), 51 deletions(-) (limited to 'libflash') diff --git a/libflash/blocklevel.c b/libflash/blocklevel.c index 9f02ee1..b827045 100644 --- a/libflash/blocklevel.c +++ b/libflash/blocklevel.c @@ -329,53 +329,95 @@ out_free: return rc; } -static int insert_bl_prot_range(struct blocklevel_range *ranges, struct bl_prot_range range) +static bool insert_bl_prot_range(struct blocklevel_range *ranges, struct bl_prot_range range) { - struct bl_prot_range *new_ranges; - struct bl_prot_range *old_ranges = ranges->prot; - int i, count = ranges->n_prot; + int i; + uint32_t pos, len; + struct bl_prot_range *prot = ranges->prot; - /* Try to merge into an existing range */ - for (i = 0; i < count; i++) { - if (!(range.start + range.len == old_ranges[i].start || - old_ranges[i].start + old_ranges[i].len == range.start)) - continue; + pos = range.start; + len = range.len; - if (range.start + range.len == old_ranges[i].start) - old_ranges[i].start = range.start; + if (len == 0) + return true; - old_ranges[i].len += range.len; + /* Check for overflow */ + if (pos + len < len) + return false; - /* - * Check the inserted range isn't wedged between two ranges, if it - * is, merge as well - */ - i++; - if (i < count && range.start + range.len == old_ranges[i].start) { - old_ranges[i - 1].len += old_ranges[i].len; + for (i = 0; i < ranges->n_prot && len > 0; i++) { + if (prot[i].start <= pos && prot[i].start + prot[i].len >= pos + len) { + len = 0; + break; /* Might as well, the next two conditions can't be true */ + } - for (; i + 1 < count; i++) - old_ranges[i] = old_ranges[i + 1]; - ranges->n_prot--; + /* Can easily extend this down just by adjusting start */ + if (pos <= prot[i].start && pos + len >= prot[i].start) { + prot[i].len += prot[i].start - pos; + prot[i].start = pos; + pos += prot[i].len; + if (prot[i].len >= len) + len = 0; + else + len -= prot[i].len; } - return 0; + /* + * Jump over this range but the new range might be so big that + * theres a chunk after + */ + if (pos >= prot[i].start && pos < prot[i].start + prot[i].len) { + if (prot[i].start + prot[i].len - pos > len) { + len -= prot[i].start + prot[i].len - pos; + pos = prot[i].start + prot[i].len; + } else { + len = 0; + } + } + /* + * This condition will be true if the range is smaller than + * the current range, therefore it should go here! + */ + if (pos < prot[i].start && pos + len <= prot[i].start) + break; } - if (ranges->n_prot == ranges->total_prot) { - new_ranges = realloc(ranges->prot, sizeof(range) * ((ranges->n_prot) + PROT_REALLOC_NUM)); - if (new_ranges) + if (len) { + int insert_pos = i; + struct bl_prot_range *new_ranges = ranges->prot; + if (ranges->n_prot == ranges->total_prot) { + new_ranges = realloc(ranges->prot, + sizeof(range) * ((ranges->n_prot) + PROT_REALLOC_NUM)); + if (!new_ranges) + return false; ranges->total_prot += PROT_REALLOC_NUM; - } else { - new_ranges = old_ranges; - } - if (new_ranges) { - memcpy(new_ranges + ranges->n_prot, &range, sizeof(range)); + } + if (insert_pos != ranges->n_prot) + for (i = ranges->n_prot; i > insert_pos; i--) + memcpy(&new_ranges[i], &new_ranges[i - 1], sizeof(range)); + range.start = pos; + range.len = len; + memcpy(&new_ranges[insert_pos], &range, sizeof(range)); ranges->prot = new_ranges; ranges->n_prot++; } - return !new_ranges; + /* Probably only worth mergeing when we're low on space */ + if (ranges->n_prot + 1 == ranges->total_prot) { + /* Check to see if we can merge ranges */ + for (i = 0; i < ranges->n_prot - 1; i++) { + if (prot[i].start + prot[i].len == prot[i + 1].start) { + int j; + prot[i].len += prot[i + 1].len; + for (j = i + 1; j < ranges->n_prot - 1; j++) + memcpy(&prot[j] , &prot[j + 1], sizeof(range)); + ranges->n_prot--; + i--; /* Maybe the next one can merge too */ + } + } + } + + return true; } int blocklevel_ecc_protect(struct blocklevel_device *bl, uint32_t start, uint32_t len) @@ -387,11 +429,7 @@ int blocklevel_ecc_protect(struct blocklevel_device *bl, uint32_t start, uint32_ */ struct bl_prot_range range = { .start = start, .len = len }; - /* - * Refuse to add regions that are already protected or are partially - * protected - */ - if (len < BYTES_PER_ECC || ecc_protected(bl, start, len)) + if (len < BYTES_PER_ECC) return -1; - return insert_bl_prot_range(&bl->ecc_prot, range); + return !insert_bl_prot_range(&bl->ecc_prot, range); } diff --git a/libflash/libffs.c b/libflash/libffs.c index 8b1c2a7..0fda97d 100644 --- a/libflash/libffs.c +++ b/libflash/libffs.c @@ -196,7 +196,7 @@ int ffs_init(uint32_t offset, uint32_t max_size, struct blocklevel_device *bl, if (ecc) { rc = blocklevel_ecc_protect(bl, start, total_size); if (rc) { - FL_ERR("Failed to blocklevel_ecc_protect(0x%08x, 0x%08x)\n", + FL_ERR("FFS: Failed to blocklevel_ecc_protect(0x%08x, 0x%08x)\n", start, total_size); goto out; } diff --git a/libflash/test/test-blocklevel.c b/libflash/test/test-blocklevel.c index 95669dc..2f0f77b 100644 --- a/libflash/test/test-blocklevel.c +++ b/libflash/test/test-blocklevel.c @@ -44,6 +44,10 @@ int main(void) ERR("Failed to blocklevel_ecc_protect(0x3000, 0x1000)\n"); return 1; } + if (blocklevel_ecc_protect(bl, 0x2f00, 0x1100)) { + ERR("Failed to blocklevel_ecc_protect(0x2f00, 0x1100)\n"); + return 1; + } /* Zero length protection */ if (!blocklevel_ecc_protect(bl, 0x4000, 0)) { @@ -58,14 +62,20 @@ int main(void) } /* Deal with overlapping protections */ - if (!blocklevel_ecc_protect(bl, 0x100, 0x1000)) { - ERR("Shouldn't have succeeded blocklevel_ecc_protect(0x100, 0x1000)\n"); + if (blocklevel_ecc_protect(bl, 0x100, 0x1000)) { + ERR("Failed to protect overlaping region blocklevel_ecc_protect(0x100, 0x1000)\n"); + return 1; + } + + /* Deal with overflow */ + if (!blocklevel_ecc_protect(bl, 1, 0xFFFFFFFF)) { + ERR("Added an 'overflow' protection blocklevel_ecc_protect(1, 0xFFFFFFFF)\n"); return 1; } - /* Deal with protections greater than max size */ - if (!blocklevel_ecc_protect(bl, 0, 0xFFFFFFFF)) { - ERR("Failed to blocklevel_ecc_protect(0, 0xFFFFFFFF)\n"); + /* Protect everything */ + if (blocklevel_ecc_protect(bl, 0, 0xFFFFFFFF)) { + ERR("Couldn't protect everything blocklevel_ecc_protect(0, 0xFFFFFFFF)\n"); return 1; } @@ -84,17 +94,30 @@ int main(void) return 1; } - if (ecc_protected(bl, 0x1000, 0) != 0) { + /* Clear the protections */ + bl->ecc_prot.n_prot = 0; + /* Reprotect */ + if (blocklevel_ecc_protect(bl, 0x3000, 0x1000)) { + ERR("Failed to blocklevel_ecc_protect(0x3000, 0x1000)\n"); + return 1; + } + /* Deal with overlapping protections */ + if (blocklevel_ecc_protect(bl, 0x100, 0x1000)) { + ERR("Failed to protect overlaping region blocklevel_ecc_protect(0x100, 0x1000)\n"); + return 1; + } + + if (ecc_protected(bl, 0x1000, 0) != 1) { ERR("Invalid result for ecc_protected(0x1000, 0)\n"); return 1; } - if (ecc_protected(bl, 0x1000, 0x1000) != 0) { + if (ecc_protected(bl, 0x1000, 0x1000) != -1) { ERR("Invalid result for ecc_protected(0x1000, 0x1000)\n"); return 1; } - if (ecc_protected(bl, 0x1000, 0x100) != 0) { + if (ecc_protected(bl, 0x1000, 0x100) != 1) { ERR("Invalid result for ecc_protected(0x1000, 0x100)\n"); return 1; } @@ -104,7 +127,7 @@ int main(void) return 1; } - if (ecc_protected(bl, 0x4000, 1) != 1) { + if (ecc_protected(bl, 0x4000, 1) != 0) { ERR("Invalid result for ecc_protected(0x4000, 1)\n"); return 1; } @@ -136,6 +159,11 @@ int main(void) return 1; } + if (blocklevel_ecc_protect(bl, 0x4f00, 0x100)) { + ERR("Failed to blocklevel_ecc_protected(0x4900, 0x100)\n"); + return 1; + } + if (blocklevel_ecc_protect(bl, 0x4900, 0x100)) { ERR("Failed to blocklevel_ecc_protected(0x4900, 0x100)\n"); return 1; @@ -146,8 +174,8 @@ int main(void) return 1; } - if (!blocklevel_ecc_protect(bl, 0x5290, 0x10)) { - ERR("Shouldn't have been able to blocklevel_ecc_protect(0x5290, 0x10)\n"); + if (blocklevel_ecc_protect(bl, 0x5290, 0x10)) { + ERR("Failed to blocklevel_ecc_protect(0x5290, 0x10)\n"); return 1; } @@ -166,7 +194,9 @@ int main(void) ERR("Failed to blocklevel_ecc_protect(0x6100, 0x100)\n"); return 1; } - + /* Make sure we trigger the merging code */ + for (i = bl->ecc_prot.n_prot; i < bl->ecc_prot.total_prot; i++) + blocklevel_ecc_protect(bl, 0x10000 + i * 0x200, 0x10); /* Check that the region merging works */ for (i = 0; i < bl->ecc_prot.n_prot - 1; i++) { if (bl->ecc_prot.prot[i].start + bl->ecc_prot.prot[i].len == bl->ecc_prot.prot[i + 1].start || -- cgit v1.1