[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