commit e247710814227a7122915ff44474e0c8aa26fd3a

Author: Samuel Mendoza-Jonas <sam@mendozajonas.com>

Date:   Wed Nov 21 17:16:01 2018 +1100

    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>

diff --git a/libflash/blocklevel.c b/libflash/blocklevel.c

index bbcc8e61..5b57d3c6 100644

--- a/libflash/blocklevel.c

+++ b/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;

 }

diff --git a/libflash/test/test-blocklevel.c b/libflash/test/test-blocklevel.c

index 42ad146a..a67f5888 100644

--- a/libflash/test/test-blocklevel.c

+++ b/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 */