[Skiboot] [PATCH] libflash/blocklevel: Allow double ecc protecting a region

Cyril Bur cyril.bur at au1.ibm.com
Thu Jul 7 13:24:30 AEST 2016


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.

Signed-off-by: Cyril Bur <cyril.bur at au1.ibm.com>
---
Fixes issue exposed by Alistairs patch: 74ba834 ("flash: Use blocklevel to do ECC reads")

 libflash/blocklevel.c | 87 ++++++++++++++++++++++++++-------------------------
 libflash/libffs.c     |  2 +-
 2 files changed, 46 insertions(+), 43 deletions(-)

diff --git a/libflash/blocklevel.c b/libflash/blocklevel.c
index 9591194..f39531a 100644
--- a/libflash/blocklevel.c
+++ b/libflash/blocklevel.c
@@ -327,50 +327,57 @@ 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 pos, len, i;
+	bool found = false;
+	struct bl_prot_range *new_ranges = 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;
-
-		if (range.start + range.len == old_ranges[i].start)
-			old_ranges[i].start = range.start;
-
-		old_ranges[i].len += range.len;
-
-		/*
-		 * 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;
+	pos = range.start;
+	len = range.len;
+	for (i = 0; i < ranges->n_prot && !found; i++) {
+		/* Fits entirely within the range */
+		if (ranges->prot[i].start <= pos &&
+				ranges->prot[i].start + ranges->prot[i].len >= pos + len) {
+			found = true;
+			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--;
+		if (ranges->prot[i].start <= pos &&
+			ranges->prot[i].start + ranges->prot[i].len > pos) {
+			ranges->prot[i].len += len;
+			found = true;
+			if (++i == ranges->n_prot) break;
 		}
 
-		return 0;
+		if (ranges->prot[i].start >= pos &&
+			ranges->prot[i].start < pos + len) {
+			/* If it is adjacent to two ranges */
+			if (found) {
+				int j;
+				ranges->prot[i - 1].len += len + ranges->prot[i].len;
+				for (j = i; j < ranges->n_prot - 1; j++)
+					memcpy(&ranges->prot[j] , &ranges->prot[j + 1], sizeof(range));
+				ranges->n_prot--;
+			} else {
+				ranges->prot[i].start = pos;
+				ranges->prot[i].len += len;
+				found = true;
+			}
+		}
 	}
 
-	if (ranges->n_prot == ranges->total_prot) {
-		new_ranges = realloc(ranges->prot, sizeof(range) * ((ranges->n_prot) + PROT_REALLOC_NUM));
-		if (new_ranges)
-			ranges->total_prot += PROT_REALLOC_NUM;
-	} else {
-		new_ranges = old_ranges;
-	}
-	if (new_ranges) {
-		memcpy(new_ranges + ranges->n_prot, &range, sizeof(range));
-		ranges->prot = new_ranges;
-		ranges->n_prot++;
+	if (!found) {
+		if (ranges->n_prot == ranges->total_prot) {
+			new_ranges = realloc(ranges->prot, sizeof(range) * ((ranges->n_prot) + PROT_REALLOC_NUM));
+			if (new_ranges)
+				ranges->total_prot += PROT_REALLOC_NUM;
+		}
+		if (new_ranges) {
+			memcpy(&new_ranges[ranges->n_prot], &range, sizeof(range));
+			ranges->prot = new_ranges;
+			ranges->n_prot++;
+		}
 	}
 
 	return !new_ranges;
@@ -385,11 +392,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);
 }
diff --git a/libflash/libffs.c b/libflash/libffs.c
index 8134962..97b48c4 100644
--- a/libflash/libffs.c
+++ b/libflash/libffs.c
@@ -194,7 +194,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;
 				}
-- 
2.9.0



More information about the Skiboot mailing list