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

[Michael Neuling]

On Thu, 2016-07-07 at 13:24 +1000, Cyril Bur wrote:
> 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>

Stewart,

So this works for me but it depends on Alistairs patch (74ba834) being
reapplied to mainline.

Acked-by: Michael Neuling <mikey at neuling.org>

Mikey

> ---
> 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;
>  				}