[Skiboot] [PATCH] pflash: Support encoding/decoding ECC'd partitions
Stewart Smith
stewart at linux.ibm.com
Tue Dec 11 16:25:05 AEDT 2018
Samuel Mendoza-Jonas <sam at mendozajonas.com> writes:
> 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?
Umm.. good point.
--
Stewart Smith
OPAL Architect, IBM.
More information about the Skiboot
mailing list