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

[Stewart Smith]

Samuel Mendoza-Jonas <sam at mendozajonas.com> writes:
> On Tue, 2018-12-11 at 15:29 +1100, Stewart Smith wrote:
>> Samuel Mendoza-Jonas <sam at mendozajonas.com> writes:
>> > 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?
>> 
>> The blocklevel_write() call will add ECC based on what's been
>> blocklevel_ecc_protect()ed, and we don't have a
>> blocklevel_write_but_i_did_ecc_already() call to bypass it... so this is
>> a bit of a hack I guess. I do want to remove it eventually though.
>
> Ah I see, we're kind of pretending it's not ECC at write time.. sounds
> about right :)

Yeah - and that's the *least* horrible part of this entire endeavour :)

-- 
Stewart Smith
OPAL Architect, IBM.