[Skiboot] [PATCH] pflash: Support for clean_on_ecc_error flag
anoo
anoo at linux.vnet.ibm.com
Wed Dec 13 06:33:17 AEDT 2017
Thanks Cyril. I got pointed by the hostboot team to this header file:
https://github.com/open-power/hostboot/blob/60ccd2d1e7876d6e408ca65d7c8ec320e3aca1fe/src/usr/pnor/common/ffs_hb.H#L66
which is then used to create the pnor xml:
https://github.com/open-power/pnor/blob/4c844a5ef7efc6605c4c877b6c92cf0a1a0df0b5/p9Layouts/defaultPnorLayout_128.xml#L56
This clear ecc flag is the only one that was missing as of today,
although the hostboot team (Dan) has raised a concern where they'd like
to add flags and have those values automatically make their wait to the
ffs code instead of having to notify us and manually add them, something
to continue discussing in the "Rework flash TOC generation" thread.
Will resubmit a second version of this patch shortly to address the unit
test error Stewart noted.
Thanks.
On 2017-12-04 16:48, Cyril Bur wrote:
> On Mon, 2017-12-04 at 15:26 -0600, Adriana Kobylak wrote:
>> Add the misc flag clear_on_ecc_error to libflash/pflash. This was
>> the only missing flag. The generator of the virtual pnor image
>> relies on libflash/pflash to provide the partition information,
>> so all flags are needed to build an accurate virtual pnor partition
>> table.
>>
>
> Hi Adriana,
>
> Thanks for the patch. I wrote all of this by simply looking at hostboot
> source code and the current pnor creation method, I must have missed
> this flag.
>
> Is there a piece of documentation where this is all clearly states all
> these flags and their intended purposes? If we're going to have
> libflash do all the work, this documentation should be made available
> to all pflash developers.
>
> Thanks,
>
> Cyril
>
>
> Reviewed-by: Cyril Bur <cyril.bur at au1.ibm.com>
>
>> Signed-off-by: Adriana Kobylak <anoo at linux.vnet.ibm.com>
>> ---
>> external/ffspart/ffspart.c | 3 +++
>> external/pflash/pflash.c | 16 +++++++++-------
>> external/pflash/test/files/06-miscprint.ffs | 1 +
>> external/pflash/test/results/01-info.out | 17 +++++++++--------
>> external/pflash/test/results/06-miscprint.out | 9 +++++++++
>> libflash/ffs.h | 1 +
>> libflash/libffs.c | 3 +--
>> libflash/libffs.h | 1 +
>> 8 files changed, 34 insertions(+), 17 deletions(-)
>>
>> diff --git a/external/ffspart/ffspart.c b/external/ffspart/ffspart.c
>> index a477018..0fe5840 100644
>> --- a/external/ffspart/ffspart.c
>> +++ b/external/ffspart/ffspart.c
>> @@ -298,6 +298,9 @@ int main(int argc, char *argv[])
>> case 'L':
>> user.miscflags |= FFS_MISCFLAGS_VOLATILE;
>> break;
>> + case 'C':
>> + user.miscflags |= FFS_MISCFLAGS_CLEARECC;
>> + break;
>> /* Not sure these are valid */
>> case 'B':
>> user.miscflags |= FFS_MISCFLAGS_BACKUP;
>> diff --git a/external/pflash/pflash.c b/external/pflash/pflash.c
>> index a5e7bc3..b3ceef0 100644
>> --- a/external/pflash/pflash.c
>> +++ b/external/pflash/pflash.c
>> @@ -115,13 +115,14 @@ static uint32_t print_ffs_info(struct ffs_handle
>> *ffsh, uint32_t toc)
>> goto out;
>> }
>>
>> - l = asprintf(&flags, "[%c%c%c%c%c%c]",
>> + l = asprintf(&flags, "[%c%c%c%c%c%c%c]",
>> has_ecc(ent) ? 'E' : '-',
>> has_flag(ent, FFS_MISCFLAGS_PRESERVED) ? 'P' : '-',
>> has_flag(ent, FFS_MISCFLAGS_READONLY) ? 'R' : '-',
>> has_flag(ent, FFS_MISCFLAGS_BACKUP) ? 'B' : '-',
>> has_flag(ent, FFS_MISCFLAGS_REPROVISION) ? 'F' : '-',
>> - has_flag(ent, FFS_MISCFLAGS_VOLATILE) ? 'V' : '-');
>> + has_flag(ent, FFS_MISCFLAGS_VOLATILE) ? 'V' : '-',
>> + has_flag(ent, FFS_MISCFLAGS_CLEARECC) ? 'C' : '-');
>> if (l < 0)
>> goto out;
>>
>> @@ -168,9 +169,9 @@ static void print_flash_info(struct flash_details
>> *flash)
>> printf("Flash info:\n");
>> printf("-----------\n");
>> printf("Name = %s\n", flash->name);
>> - printf("Total size = %" PRIu64 "MB\t Flags E:ECC, P:PRESERVED,
>> R:READONLY\n",
>> - flash->total_size >> 20);
>> - printf("Erase granule = %2d%-13sB:BACKUP, F:REPROVISION,
>> V:VOLATILE\n",
>> + printf("Total size = %" PRIu64 "MB\t Flags E:ECC, P:PRESERVED,
>> R:READONLY, "
>> + "B:BACKUP\n", flash->total_size >> 20);
>> + printf("Erase granule = %2d%-13sF:REPROVISION, V:VOLATILE,
>> C:CLEARECC\n",
>> flash->erase_granule >> 10, "KB");
>>
>> if (bmc_flash)
>> @@ -581,13 +582,14 @@ static void print_partition_detail(struct
>> ffs_handle *ffsh, uint32_t part_id)
>>
>> printf("Flags:\n");
>>
>> - l = asprintf(&flags, "%s%s%s%s%s%s", has_ecc(ent) ? "ECC [E]\n" :
>> "",
>> + l = asprintf(&flags, "%s%s%s%s%s%s%s", has_ecc(ent) ? "ECC [E]\n" :
>> "",
>> has_flag(ent, FFS_MISCFLAGS_PRESERVED) ? "PRESERVED [P]\n" : "",
>> has_flag(ent, FFS_MISCFLAGS_READONLY) ? "READONLY [R]\n" : "",
>> has_flag(ent, FFS_MISCFLAGS_BACKUP) ? "BACKUP [B]\n" : "",
>> has_flag(ent, FFS_MISCFLAGS_REPROVISION) ?
>> "REPROVISION [F]\n" : "",
>> - has_flag(ent, FFS_MISCFLAGS_VOLATILE) ? "VOLATILE [V]\n" : "");
>> + has_flag(ent, FFS_MISCFLAGS_VOLATILE) ? "VOLATILE [V]\n" : "",
>> + has_flag(ent, FFS_MISCFLAGS_CLEARECC) ? "CLEARECC [C]\n" : "");
>> if (l < 0) {
>> fprintf(stderr, "Memory allocation failure printing flags!\n");
>> goto out;
>> diff --git a/external/pflash/test/files/06-miscprint.ffs
>> b/external/pflash/test/files/06-miscprint.ffs
>> index 461e408..353ceb0 100644
>> --- a/external/pflash/test/files/06-miscprint.ffs
>> +++ b/external/pflash/test/files/06-miscprint.ffs
>> @@ -3,3 +3,4 @@ READONLY,0x0004000,0x1000,R,/dev/zero
>> REPROVISION,0x5000,0x1000,F,/dev/zero
>> BACKUP,0x000006000,0x1000,B,/dev/zero
>> VOLATILE,0x000007000,0x1000,L,/dev/zero
>> +CLEARECC,0x000008000,0x1000,L,/dev/zero
>> diff --git a/external/pflash/test/results/01-info.out
>> b/external/pflash/test/results/01-info.out
>> index 628ddcc..204d594 100644
>> --- a/external/pflash/test/results/01-info.out
>> +++ b/external/pflash/test/results/01-info.out
>> @@ -1,14 +1,15 @@
>> Flash info:
>> -----------
>> Name = FILE
>> -Total size = 0MB Flags E:ECC, P:PRESERVED, R:READONLY
>> -Erase granule = 0KB B:BACKUP, F:REPROVISION, V:VOLATILE
>> +Total size = 0MB Flags E:ECC, P:PRESERVED, R:READONLY, B:BACKUP
>> +Erase granule = 0KB F:REPROVISION, V:VOLATILE, C:CLEARECC
>>
>> TOC at 0x00000000 Partitions:
>> -----------
>> -ID=00 part 0x00000000..0x00001000 (actual=0x00001000)
>> [------]
>> -ID=01 ONE 0x00003000..0x00004000 (actual=0x00001000)
>> [E-----]
>> -ID=02 TWO 0x00004000..0x00005000 (actual=0x00001000)
>> [E---F-]
>> -ID=03 THREE 0x00005000..0x00006000 (actual=0x00001000)
>> [E---F-]
>> -ID=04 FOUR 0x00006000..0x00007000 (actual=0x00001000)
>> [E---F-]
>> -ID=05 FIVE 0x00007000..0x00008000 (actual=0x00001000)
>> [-----V]
>> +ID=00 part 0x00000000..0x00001000 (actual=0x00001000)
>> [-------]
>> +ID=01 ONE 0x00003000..0x00004000 (actual=0x00001000)
>> [E------]
>> +ID=02 TWO 0x00004000..0x00005000 (actual=0x00001000)
>> [E---F--]
>> +ID=03 THREE 0x00005000..0x00006000 (actual=0x00001000)
>> [E---F--]
>> +ID=04 FOUR 0x00006000..0x00007000 (actual=0x00001000)
>> [E---F--]
>> +ID=05 FIVE 0x00007000..0x00008000 (actual=0x00001000)
>> [-----V-]
>> +ID=06 SIX 0x00008000..0x00009000 (actual=0x00001000)
>> [------C]
>> diff --git a/external/pflash/test/results/06-miscprint.out
>> b/external/pflash/test/results/06-miscprint.out
>> index 9ee2af6..f28057d 100644
>> --- a/external/pflash/test/results/06-miscprint.out
>> +++ b/external/pflash/test/results/06-miscprint.out
>> @@ -43,3 +43,12 @@ Start End Actual
>>
>> Flags:
>> VOLATILE [V]
>> +Detailed partition information
>> +Name:
>> +CLEARECC (ID=06)
>> +
>> +Start End Actual
>> +0x00008000 0x00009000 0x00001000
>> +
>> +Flags:
>> +CLEARECC [C]
>> diff --git a/libflash/ffs.h b/libflash/ffs.h
>> index 26cb9d8..433ecac 100644
>> --- a/libflash/ffs.h
>> +++ b/libflash/ffs.h
>> @@ -77,6 +77,7 @@ enum ffs_type {
>> #define FFS_MISCFLAGS_BACKUP 0x20
>> #define FFS_MISCFLAGS_REPROVISION 0x10
>> #define FFS_MISCFLAGS_VOLATILE 0x08
>> +#define FFS_MISCFLAGS_CLEARECC 0x04
>>
>> /**
>> * struct __ffs_entry_user - On flash user data entries
>> diff --git a/libflash/libffs.c b/libflash/libffs.c
>> index 87c5197..57dca49 100644
>> --- a/libflash/libffs.c
>> +++ b/libflash/libffs.c
>> @@ -724,8 +724,7 @@ int ffs_entry_user_set(struct ffs_entry *ent,
>> struct ffs_entry_user *user)
>> return -1;
>> if (user->miscflags & ~(FFS_MISCFLAGS_PRESERVED |
>> FFS_MISCFLAGS_BACKUP |
>> FFS_MISCFLAGS_READONLY | FFS_MISCFLAGS_REPROVISION |
>> - FFS_MISCFLAGS_VOLATILE
>> -))
>> + FFS_MISCFLAGS_VOLATILE | FFS_MISCFLAGS_CLEARECC))
>> return -1;
>>
>> memcpy(&ent->user, user, sizeof(*user));
>> diff --git a/libflash/libffs.h b/libflash/libffs.h
>> index 0610399..47a3808 100644
>> --- a/libflash/libffs.h
>> +++ b/libflash/libffs.h
>> @@ -89,6 +89,7 @@ struct ffs_entry_user {
>> #define FFS_MISCFLAGS_BACKUP 0x20
>> #define FFS_MISCFLAGS_REPROVISION 0x10
>> #define FFS_MISCFLAGS_VOLATILE 0x08
>> +#define FFS_MISCFLAGS_CLEARECC 0x04
>>
>>
>> bool has_ecc(struct ffs_entry *ent);
More information about the Skiboot
mailing list