[Skiboot] [PATCH v2] ffspart: Support flashing already ECC protected images

[Samuel Mendoza-Jonas]

On Mon, 2018-12-10 at 17:48 +1100, Stewart Smith wrote:
> We do this by assuming filenames with '.ecc' in them are already ECC
> protected.
> 
> This solves a practical problem in transitioning op-build to use ffspart
> for pnor assembly rather than three perl scripts and a lot of XML.
> 
> We also update the ffspart tests to take into account ECC requirements.
> 
> Signed-off-by: Stewart Smith <stewart at linux.ibm.com>
> ---
> Changes since v1:
>   - make the test suite pass
> ---
>  external/ffspart/ffspart.c                    |  16 ++++++++++++++--
>  external/ffspart/test/files/04-tiny-pnor2.out | Bin 2560 -> 2560 bytes
>  .../ffspart/test/results/07-big-files.err     |   2 +-
>  external/ffspart/test/tests/04-tiny-pnor2     |   2 +-
>  external/ffspart/test/tests/08-small-files    |   2 +-
>  5 files changed, 17 insertions(+), 5 deletions(-)
> 
> diff --git a/external/ffspart/ffspart.c b/external/ffspart/ffspart.c
> index eeee0d4d657b..bb46a9eaf311 100644
> --- a/external/ffspart/ffspart.c
> +++ b/external/ffspart/ffspart.c
> @@ -31,6 +31,7 @@
>  #include <libflash/libflash.h>
>  #include <libflash/libffs.h>
>  #include <libflash/blocklevel.h>
> +#include <libflash/ecc.h>
>  #include <common/arch_flash.h>
>  
>  /*
> @@ -250,6 +251,14 @@ static int parse_entry(struct blocklevel_device *bl,
>  
>  	if (*line != '\0' && *(line + 1) != '\0') {
>  		filename = line + 1;
> +
> +		/*
> +		 * Support flashing already ecc'd data as this is the case
> +		 * for POWER8 SBE image binary.
> +		 */
> +		if (has_ecc(new_entry) && !strstr(filename, ".ecc"))
> +			blocklevel_ecc_protect(bl, pbase, psize);

If the entry has the ECC bit shouldn't we mark it protected regardless of
the contents of the file?

> +
>  		data_fd = open(filename, O_RDONLY);
>  		if (data_fd == -1) {
>  			fprintf(stderr, "Couldn't open file '%s' for '%s' partition "
> @@ -269,9 +278,12 @@ static int parse_entry(struct blocklevel_device *bl,
>  		 * Sanity check that the file isn't too large for
>  		 * partition
>  		 */
> +		if (has_ecc(new_entry) && !strstr(filename, ".ecc"))
> +			psize = ecc_buffer_size_minus_ecc(psize);
>  		if (pactual > psize) {

I needed to stare at this for a while but I think I'm happy with it -
checking if the size of the non-ECC file is less than the size of the
ECC-partition less the ECC bits?

> -			fprintf(stderr, "File '%s' for partition '%s' is too large\n",
> -					filename, name);
> +			fprintf(stderr, "File '%s' for partition '%s' is too large,"
> +				" %u > %u\n",
> +				filename, name, pactual, psize);
>  			close(data_fd);
>  			return -1;
>  		}
> diff --git a/external/ffspart/test/files/04-tiny-pnor2.out b/external/ffspart/test/files/04-tiny-pnor2.out
> index 394edf0411267503a24caf3bb25d4b4b0a0327e6..617c4ef444dee89012155a2540ed973988a3c5c5 100644
> GIT binary patch
> literal 2560
> zcmWG=3<_ajU|<AdW*}|=Vpa&3feXk+0RfJ|HwzMrN>BtL9OnN(0A#|<0n#Ajs0;e%
> z^>zFExuP2ivWO9CKNE-zQ2?b-R5yS<h$2J+$6%xNzmRZ$bR8@}0hs+ at w?GtNx1T`(
> zT|4po)jJG-gm?tGqJ;o3zTozQ-9jt--!0eK?dBgEgyv#4B>TZ`p_TnR7ig~o#WOX5
> rAr$-{fecVM8bQHGWPto{3<V4$kOA_)2^5S(2FU-TU^E0qatHtbT!f8<
> 
> literal 2560
> zcmWG=3<_ajU|<AdW*}|=Vpa&3feXk+0RfJ|HwzMrN>BtL9OnN(0A#|<0n#Ajs0;e%
> z^>zFExuP2ivWO9CKNE-zQ2?b-R5yS<h$2J+$6%xNzmRZ$bR8@}0hs+ at w?GtNx1T`(
> zT|4po)jJG-gm?tGqJ;o3zTozQ-9jt--!0eK?dBgEgyv#4B>TZ`p_TnR7ig~o#WOX5
> c;RpvtBODlya9}dR0X55#QL{!vV5o%v0EntXDF6Tf
> 
> diff --git a/external/ffspart/test/results/07-big-files.err b/external/ffspart/test/results/07-big-files.err
> index 083bad20ea84..bfb6fa0220da 100644
> --- a/external/ffspart/test/results/07-big-files.err
> +++ b/external/ffspart/test/results/07-big-files.err
> @@ -1,4 +1,4 @@
>  WARNING: Attempting to parse a partition line without any TOCs created.
>           Generating a default TOC at zero
> -File 'FILE_ONE' for partition 'ONE' is too large
> +File 'FILE_ONE' for partition 'ONE' is too large, 257 > 227
>  Failed to parse input file 'FILE' at line 1
> diff --git a/external/ffspart/test/tests/04-tiny-pnor2 b/external/ffspart/test/tests/04-tiny-pnor2
> index a7e79ab5e1f6..3db5f1e29c6e 100644
> --- a/external/ffspart/test/tests/04-tiny-pnor2
> +++ b/external/ffspart/test/tests/04-tiny-pnor2
> @@ -4,7 +4,7 @@ touch $DATA_DIR/$CUR_TEST.gen
>  i=1;
>  while [ $i -lt 5 ] ; do
>  	j=0;
> -	while [ $j -lt $((0x100)) ] ; do
> +	while [ $j -lt $((0xe0)) ] ; do
>  		echo -n "$i" >> $DATA_DIR/$CUR_TEST.$i;
>  		j=$(expr $j + 1);
>  	done
> diff --git a/external/ffspart/test/tests/08-small-files b/external/ffspart/test/tests/08-small-files
> index fb2a98b3dedb..1e4f3b3beba4 100644
> --- a/external/ffspart/test/tests/08-small-files
> +++ b/external/ffspart/test/tests/08-small-files
> @@ -4,7 +4,7 @@ touch $DATA_DIR/$CUR_TEST.gen
>  i=1;
>  while [ $i -lt 5 ] ; do
>  	j=0;
> -	while [ $j -lt $((0xff)) ] ; do
> +	while [ $j -lt $((0xe0)) ] ; do
>  		echo -n "$i" >> $DATA_DIR/$CUR_TEST.$i;
>  		j=$(expr $j + 1);
>  	done