[flashrom] [PATCH 2/4] layout: Add -i <image>[:<file>] support.

[David Hendricks]

On Sun, Sep 15, 2013 at 6:36 PM, Carl-Daniel Hailfinger <
c-d.hailfinger.devel.2006 at gmx.net> wrote:

> >>>>> +This is useful for example if you want to write only a part of the
> flash chip and leave everything else alone
> >>>>> +without preparing an image of the complete chip manually:
> >>>> Uhh... sounds denglish. Suggested rewording:
> >>>> This allows replacing part of a ROM file with contents from another
> file
> >>>> before writing the resulting contents to the flash chip, eliminating
> the
> >>>> separate step of manually assembling a combined ROM image.
> >>> You seem to imply that the -i parameters do only overwrite parts of the
> >>> file given by -w. While this is true for the current implementation,
> >>> David and I discussed about making the file parameter after -w
> >>> optional.
> >> Ouch! I think that is a really really bad idea. I can see reasons to
> >> avoid full image files, but it feels wrong to have undefined regions in
> >> the image. Especially if writing fails and recovery is needed.
> > There is no such thing as an undefined region (yet at least). For
> > address ranges for which no content is supplied by the user in the form
> > of an <image file> or a <region file>, the old content of the flash
> > chip is what should be there after any flashrom invocation. Cases of
> > "-w -i bla" where neither an <image file> nor a <region file> are given
> > need to be handled as errors of course.
>
> That would make an implicit read from the flash chip before any write a
> hard requirement. I hear people complaining from time to time about the
> initial chip read in flashrom, and this won't make it better.
>
> Besides that, writes where part of the image is read on-the-fly from the
> flash chip directly before writing are not reproducible if an error
> occurs. Think of the following scenario: Writing fails, and the flash
> chip is now partially corrupted due to an erase command which affected a
> larger region size than expected. As a result, parts of the implicitly
> read region are now erased. Repeating the same write command with a
> fixed flashrom version will _not_ yield the correct flash chip contents.
> User despairs, smashes the computer and takes up a career in pottery. If
> the image had been built completely from on-disk files (i.e. without
> implicit chip reads), repeating the same write command would have
> resulted in the correct flash chip contents.
>

The old contents of the flash are already read regardless. I don't see how
recovering from failure is an issue here, unless we change the default
behavior (I don't recommend doing that).

For chromeos we added a "--fast-verify" option to skip reading and
verifying non-included regions when doing particular writes. It's intended
for use in controlled circumstances (e.g. development with an external
programmer).


> > But anyway, your sentence still implies that the file given in -w is
> > modified and that's what I was concerned about actually. No idea why I
> > brought up the file-less -w at all (anymore) sorry.
>
> I see. Not sure how to express that best. Next try:
> This allows replacing the contents of one ROM image region with contents
> from another file before writing the resulting contents to the flash
> chip, eliminating the separate step of manually assembling a combined
> ROM image.
>

I'd recommend stating this in concrete terms that translate directly to
command-line usage: The contents of any -i option takes priority over the
contents of the -w option.

BTW, I tried to distill some of these rules for syntax parsing @ line 686
here:
https://chromium.googlesource.com/chromiumos/third_party/flashrom/+/master/cli_mfg.c.

Feel free to borrow any wording if it helps.


> There's another problem with the patch, though: Allowing region files
> for read (instead of just for write) does violate the principle of least
> surprise: Some users might expect regions to be written to separate
> files on disk, i.e. having -r -i as reverse operation to -w -i. Right
> now that assumption is not true.
>

Why would a user provide a filename to a -i option along with -r if they
did not intend for a file to be written to disk?

-- 
David Hendricks (dhendrix)
Systems Software Engineer, Google Inc.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.flashrom.org/pipermail/flashrom/attachments/20130917/4a862722/attachment.html>