[Skiboot] [PATCH] pflash: Support encoding/decoding ECC'd partitions

Samuel Mendoza-Jonas sam at mendozajonas.com
Tue Dec 11 15:40:18 AEDT 2018


On Tue, 2018-12-11 at 15:18 +1100, Stewart Smith wrote:
> With the new --ecc option, pflash can add/remove ECC when
> reading/writing flash partitions protected by ECC.
> 
> This is *not* flawless with current PNORs out in the wild though, as
> they do not typically fill the whole partition with valid ECC data, so
> you have to know how big the valid ECC'd data is and specify the size
> manually. Note that for some partitions this is pratically impossible
> without knowing the details of the content of the partition.
> 
> A future patch is likely to introduce an option to "stop reading data
> when ECC starts failing and assume everything is okay rather than error
> out" to support reading the "valid" data from existing PNOR images.
> 
> Signed-off-by: Stewart Smith <stewart at linux.ibm.com>
> ---
>  external/pflash/pflash.c                      | 43 ++++++++++++++-----
>  external/pflash/test/results/00-usage.out     |  4 ++
>  .../pflash/test/results/05-bad-numbers.out    |  4 ++
>  3 files changed, 41 insertions(+), 10 deletions(-)
> 
> diff --git a/external/pflash/pflash.c b/external/pflash/pflash.c
> index c81520cb9039..8cccb506f5fc 100644
> --- a/external/pflash/pflash.c
> +++ b/external/pflash/pflash.c
> @@ -36,6 +36,7 @@
>  #include <libflash/libflash.h>
>  #include <libflash/libffs.h>
>  #include <libflash/blocklevel.h>
> +#include <libflash/ecc.h>
>  #include <common/arch_flash.h>
>  #include "progress.h"
>  
> @@ -48,6 +49,7 @@ struct flash_details {
>  	uint64_t toc;
>  	uint64_t total_size;
>  	uint32_t erase_granule;
> +	bool mark_ecc;
>  };
>  
>  /* Full pflash version number (possibly includes gitid). */
> @@ -142,7 +144,7 @@ static struct ffs_handle *open_ffs(struct flash_details *flash)
>  	int rc;
>  
>  	rc = ffs_init(flash->toc, flash->total_size,
> -			flash->bl, &ffsh, 0);
> +			flash->bl, &ffsh, flash->mark_ecc);
>  	if (rc) {
>  		fprintf(stderr, "Error %d opening ffs !\n", rc);
>  		if (flash->toc) {
> @@ -429,7 +431,7 @@ static int program_file(struct blocklevel_device *bl,
>  	}
>  
>  	printf("Programming & Verifying...\n");
> -	progress_init(size >> 8);
> +	progress_init(size);
>  	while(size) {
>  		ssize_t len;
>  
> @@ -456,7 +458,7 @@ static int program_file(struct blocklevel_device *bl,
>  			goto out;
>  		}
>  		start += len;
> -		progress_tick(actual_size >> 8);
> +		progress_tick(actual_size);
>  	}
>  	progress_end();
>  
> @@ -487,7 +489,7 @@ static int do_read_file(struct blocklevel_device *bl, const char *file,
>  	printf("Reading to \"%s\" from 0x%08x..0x%08x !\n",
>  	       file, start, start + size);
>  
> -	progress_init(size >> 8);
> +	progress_init(size);
>  	while(size) {
>  		ssize_t len;
>  
> @@ -511,7 +513,7 @@ static int do_read_file(struct blocklevel_device *bl, const char *file,
>  		start += rc;
>  		size -= rc;
>  		done += rc;
> -		progress_tick(done >> 8);
> +		progress_tick(done);
>  	}
>  	progress_end();
>  	close(fd);
> @@ -690,6 +692,10 @@ static void print_help(const char *pname)
>  	printf("\t\tUsed to ECC clear a partition of the flash\n");
>  	printf("\t\tMust be used in conjunction with -P. Will erase the\n");
>  	printf("\t\tpartition and then set all the ECC bits as they should be\n\n");
> +	printf("\t-9 --ecc\n");
> +	printf("\t\tEncode/Decode ECC where specified in the FFS header.\n");
> +	printf("\t\tThis 9 byte ECC method is used for some OpenPOWER\n");
> +	printf("\t\tpartitions.\n");
>  	printf("\t-i, --info\n");
>  	printf("\t\tDisplay some information about the flash.\n\n");
>  	printf("\t--detail\n");
> @@ -706,7 +712,8 @@ int main(int argc, char *argv[])
>  	struct flash_details flash = { 0 };
>  	static struct ffs_handle *ffsh = NULL;
>  	uint32_t ffs_index;
> -	uint32_t address = 0, read_size = 0, write_size = 0, detail_id = UINT_MAX;
> +	uint32_t address = 0, read_size = 0, detail_id = UINT_MAX;
> +	uint32_t write_size = 0, write_size_minus_ecc = 0;
>  	bool erase = false, do_clear = false;
>  	bool program = false, erase_all = false, info = false, do_read = false;
>  	bool enable_4B = false, disable_4B = false;
> @@ -743,11 +750,12 @@ int main(int argc, char *argv[])
>  			{"skip",	required_argument,	NULL,	'k'},
>  			{"toc",		required_argument,	NULL,	'T'},
>  			{"clear",   no_argument,        NULL,   'c'},
> +			{"ecc",         no_argument,            NULL,   '9'},
>  			{NULL,	    0,                  NULL,    0 }
>  		};
>  		int c, oidx = 0;
>  
> -		c = getopt_long(argc, argv, "+:a:s:P:r:43Eep:fdihvbtgS:T:cF:",
> +		c = getopt_long(argc, argv, "+:a:s:P:r:43Eep:fdihvbtgS:T:c9F:",
>  				long_opts, &oidx);
>  		if (c == -1)
>  			break;
> @@ -862,6 +870,9 @@ int main(int argc, char *argv[])
>  				}
>  			}
>  			break;
> +		case '9':
> +			flash.mark_ecc = true;
> +			break;
>  		case ':':
>  			fprintf(stderr, "Unrecognised option \"%s\" to '%c'\n", optarg, optopt);
>  			no_action = true;
> @@ -1052,6 +1063,9 @@ int main(int argc, char *argv[])
>  		write_size = stbuf.st_size;
>  	}
>  
> +	/* Only take ECC into account under some conditions later */
> +	write_size_minus_ecc = write_size;
> +
>  	/* If read specified and no read_size, use flash size */
>  	if (do_read && !read_size && !part_name)
>  		read_size = flash.total_size;
> @@ -1095,18 +1109,27 @@ int main(int argc, char *argv[])
>  		/* Read size is obtained from partition "actual" size */
>  		if (!read_size)
>  			read_size = pactsize;
> +		/* If we're decoding ecc and partition is ECC'd, then adjust */
> +		if (ecc && flash.mark_ecc)
> +			read_size = (ecc_buffer_size_minus_ecc(read_size)/9)*9;
>  
>  		/* Write size is max size of partition */
>  		if (!write_size)
>  			write_size = pmaxsz;
>  
> +		/* But write size can take into account ECC as well */
> +		if (ecc && flash.mark_ecc)
> +			write_size_minus_ecc = (ecc_buffer_size_minus_ecc(write_size)/9)*9;

I'm not sure I understand this; what does the /9)*9 do that
ecc_buffer_size_minus_ecc() doesn't?

> +		else
> +			write_size_minus_ecc = write_size;
> +
>  		/* Crop write size to partition size if --force was passed */
> -		if (write_size > pmaxsz && !must_confirm) {
> +		if ((write_size_minus_ecc > pmaxsz) && !must_confirm) {
>  			printf("WARNING: Size (%d bytes) larger than partition"
>  			       " (%d bytes), cropping to fit\n",
>  			       write_size, pmaxsz);
>  			write_size = pmaxsz;
> -		} else if (write_size > pmaxsz) {
> +		} else if (write_size_minus_ecc > pmaxsz) {
>  			printf("ERROR: Size (%d bytes) larger than partition"
>  			       " (%d bytes). Use --force to force\n",
>  			       write_size, pmaxsz);
> @@ -1169,7 +1192,7 @@ int main(int argc, char *argv[])
>  		rc = erase_range(&flash, address, write_size,
>  				program, ffsh, ffs_index);
>  	if (!rc && program)
> -		rc = program_file(flash.bl, write_file, address, write_size,
> +		rc = program_file(flash.bl, write_file, address, write_size_minus_ecc,
>  				ffsh, ffs_index);
>  	if (!rc && do_clear)
>  		rc = set_ecc(&flash, address, write_size);
> diff --git a/external/pflash/test/results/00-usage.out b/external/pflash/test/results/00-usage.out
> index caf7b8d8d580..a3721ca8e3b8 100644
> --- a/external/pflash/test/results/00-usage.out
> +++ b/external/pflash/test/results/00-usage.out
> @@ -97,6 +97,10 @@ Usage: ./pflash [options] commands...
>  		Must be used in conjunction with -P. Will erase the
>  		partition and then set all the ECC bits as they should be
>  
> +	-9 --ecc
> +		Encode/Decode ECC where specified in the FFS header.
> +		This 9 byte ECC method is used for some OpenPOWER
> +		partitions.
>  	-i, --info
>  		Display some information about the flash.
>  
> diff --git a/external/pflash/test/results/05-bad-numbers.out b/external/pflash/test/results/05-bad-numbers.out
> index 17db6650f591..2a76c28c0186 100644
> --- a/external/pflash/test/results/05-bad-numbers.out
> +++ b/external/pflash/test/results/05-bad-numbers.out
> @@ -97,6 +97,10 @@ Usage: ./pflash [options] commands...
>  		Must be used in conjunction with -P. Will erase the
>  		partition and then set all the ECC bits as they should be
>  
> +	-9 --ecc
> +		Encode/Decode ECC where specified in the FFS header.
> +		This 9 byte ECC method is used for some OpenPOWER
> +		partitions.
>  	-i, --info
>  		Display some information about the flash.
>  




More information about the Skiboot mailing list