libflash: Don't merge ECC-protected ranges

Libflash currently merges contiguous ECC-protected ranges, but doesn't
check that the ECC bytes at the end of the first and start of the second
range actually match sanely. More importantly, if blocklevel_read() is
called with a position at the start of a partition that is contained
somewhere within a region that has been merged it will update the
position assuming ECC wasn't being accounted for. This results in the
position being somewhere well after the actual start of the partition
which is incorrect.

For now, remove the code merging ranges. This means more ranges must be
held and checked however it prevents incorrectly reading ECC-correct
regions like below:

[  174.334119453,7] FLASH: CAPP partition has ECC
[  174.437349574,3] ECC: uncorrectable error: ffffffffffffffff ff
[  174.437426306,3] FLASH: failed to read the first 0x1000 from CAPP partition, rc 14
[  174.439919343,3] CAPP: Error loading ucode lid. index=201d1

Signed-off-by: Samuel Mendoza-Jonas <sam@mendozajonas.com>
Signed-off-by: Stewart Smith <stewart@linux.ibm.com>

libflash/blocklevel.c

@@ -53,9 +53,9 @@ static int ecc_protected(struct blocklevel_device *bl, uint64_t pos, uint64_t le
 		}
 
 		/*
-		 * Since we merge regions on inserting we can be sure that a
-		 * partial fit means that the non fitting region won't fit in another ecc
-		 * region
+		 * Even if ranges are merged we can't currently guarantee two
+		 * contiguous regions are sanely ECC protected so a partial fit
+		 * is no good.
 		 */
 		if ((bl->ecc_prot.prot[i].start >= pos && bl->ecc_prot.prot[i].start < pos + len) ||
 		   (bl->ecc_prot.prot[i].start <= pos &&
@@ -696,26 +696,6 @@ static bool insert_bl_prot_range(struct blocklevel_range *ranges, struct bl_prot
 		prot = new_ranges;
 	}
 
-	/* Probably only worth mergeing when we're low on space */
-	if (ranges->n_prot + 1 == ranges->total_prot) {
-		FL_DBG("%s: merging ranges\n", __func__);
-		/* 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;
-				FL_DBG("%s: merging 0x%" PRIx64 "..0x%" PRIx64 " with "
-						"0x%" PRIx64 "..0x%" PRIx64 "\n",
-						__func__, prot[i].start, prot[i].start + prot[i].len,
-						prot[i + 1].start, prot[i + 1].start + prot[i + 1].len);
-				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;
 }
 

libflash/test/test-blocklevel.c

@@ -289,24 +289,6 @@ int main(void)
 		ERR("Failed to blocklevel_ecc_protect(0x6200, 0x100)\n");
 		return 1;
 	}
-	/*This addition should cause this one to merge the other two together*/
-	if (blocklevel_ecc_protect(bl, 0x6100, 0x100)) {
-		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 ||
-			  bl->ecc_prot.prot[i + 1].start + bl->ecc_prot.prot[i + 1].len == bl->ecc_prot.prot[i].start) {
-			ERR("Problem with protection range merge code, region starting at 0x%08lx for 0x%08lx appears "
-				"to touch region 0x%lx for 0x%lx\n", bl->ecc_prot.prot[i].start, bl->ecc_prot.prot[i].len,
-				bl->ecc_prot.prot[i + 1].start, bl->ecc_prot.prot[i + 1].len);
-			return 1;
-		}
-	}
 
 	/* Test ECC reading and writing being 100% transparent to the
 	 * caller */