[flashrom] [PATCH 2/4] layout: Add -i <image>[:<file>] support.
c-d.hailfinger.devel.2006 at gmx.net
Tue Sep 17 10:55:37 CEST 2013
Am 16.09.2013 15:52 schrieb Stefan Tauner:
> On Mon, 16 Sep 2013 03:36:47 +0200
> Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net> wrote:
>> Am 16.09.2013 02:40 schrieb Stefan Tauner:
>>> On Mon, 16 Sep 2013 00:46:48 +0200
>>> Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net> wrote:
>>>> Am 15.09.2013 20:14 schrieb Stefan Tauner:
>>>>> On Sun, 15 Sep 2013 04:15:44 +0200
>>>>> Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net> wrote:
>>>>>> Am 12.09.2013 22:40 schrieb Stefan Tauner:
>>> Then there is the "image problem":
>>>>>>> -are hexadecimal addresses within the ROM file and do not refer to any
>>>>>>> -physical address. Please note that using a 0x prefix for those hexadecimal
>>>>>>> +are hexadecimal addresses within the ROM file representing the flash ROM contents.
>>>>>> are hexadecimal addresses within the virtual file representing the flash
>>>>>> chip contents
>>>>> So... "virtual file representing the flash chip contents" seems to be
>>>>> the new wording for... that. We should try to come up with a shorter
>>>>> way to refer to that. (VFRTFCC hm... nope. :)
>>>> "ROM image"? "ROM file"? I do prefer "ROM image".
>>>> Suggested explanation to be added somewhere in the man page:
>>>> "For historical reasons, the contents read from or written to a flash
>>>> chip are called ROM image. Addresses within a ROM image start at 0
>>>> regardless of the address where the flash chip is mapped in hardware."
>>> If we can avoid using "ROM" in relation to layouts we should avoid it
>>> here too IMO. The main problem here is to distinguish the VFRTFCC (aka
>>> old_- and new_contents or the concept behind that), from the image files
>>> of whole chips, and from the files used to supply/retrieve region
>>> - no idea regarding VFRTFCC, maybe we can get away with paraphrasing it
>>> - image file for files containing the content of a whole flash chip
>>> - region file? like above but for address ranges/regions only
>>> Till now I was thinking of image files as images reflecting the
>>> contents of flash... but also if they only reflect a part of the flash,
>>> i.e. if they are really <region files>. Using it for the "big" files
>>> only may provoke confusion(?)
>> I still like "ROM image" for the complete image. That phrase is actually
>> usable both for the full on-disk image file ("ROM image file") as well
>> as the in-memory and possibly rebuilt/reassembled image ("in-memory ROM
>> image" or somesuch).
> That's the/my problem with the word image: It may be understood as
> filesystem file too even if only the in-memory image is meant. Since
> the filesystem file is not necessarily equal to the in-memory image (in
> case of -i options being present), I would like to clearly distinguish
> Regarding ROM I have the same problems here as above in "ROM layout",
> although here it is less severe, because it does not allow for so many
> interpretations with the given context. So I think I could very well
> live with "ROM image" for VFRTFCC and "ROM image file" for the files
> named by -r/-w/-v parameters. I would still prefer an alternative if we
> can come up with one that. chip (or flash) image (file), complete or
> (full) image (file), chip/flash content (file), ...
Maybe someone else can weigh in. Otherwise I'd say we use what we have
at this point in the discussion.
>> What about "partial image" or "region image" for files specified with
>> --include region:file?
> For those the image problem stated above is not so severe, but we
> should distinguish them too to aid understanding of the differences of
> the whole-chip images vs. files, else region image would be fine.
sub-image? chunk image? region image? partial image? Lots of possible
names come to mind. "region image file" has the advantage of making it
clear that we're dealing with a file, and that it has something to do
>>>>>>> +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
>>>> 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.
> It does not make it more required than now. The absolute minimum is
> that we know what any touched block should contain eventually before we
> start. This does not allow for any overall diagnostics or recovery in
> any case anyway, and the infrastructure patch that follows in this
> patch set has this property too. You should not add a switch to dump the
> cake and be surprised if no one is able to eat it after pressing the
> button - meaning that we can not support fancy recovery and diagnostics
> when the user wants to skip superfluous chip reads (pre-reads and
> post-write verifications). That does not mean we should force him to,
> nor that we should design flashrom's architecture for only one of the
> two cases. Both leads to headaches when someone starts to disagree and
> forks flashrom - think of multi-billion transnational companies. ;)
>> 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.
> This is not at all related to what we are discussing. We do not write
> out the read image yet in case of errors either (nor do we try to write
> that back in emergencies). This should change anyway because in some
> configurations we are already not able to recover - yes we can
> reproduce the problem, but users still end up in pottery business from
> time to time because of that (e.g. thinkpads with unrecoverable
> unit-specific data in flash). Additionally we could then write out the
> complete image flashrom tried to place into the flash chip to be able
> to reproduce problems too. Please let us ignore that whole discussion
> until we really have to have it. No one wants to build a wall (yet ;).
Agreed, let's postpone this.
>>> 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.
> Not sure if one would understand the subtle difference without knowing
> it beforehand. I think the use of region there might even confuse more
> than it helps. The context of this sentence is the explanation of
> the :<file> parameter for --include:
> The optional
> .B file
> parameter specifies the name of a file that is used to map the contents of that very region only.
That's also not really an optimal way to express the parameter. Strictly
speaking, the file is not used to map contents of the region, but the
other way round (mostly).
> What about this:
> This allows the contents of a part of the $romimage to be overlayed with
> content from another file when writing.
Mh. How about:
The optional file parameter causes the contents of this region to be
replaced by the contents of the file specified here.
> We need to change that part of the documentation anyway when we add -r
> and -v support for --include.
Right. This also means we have to disallow --include for -r until -r
support is in. Does it even make sense to support --include for -v?
>> 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.
> Hm, eventually that should be the case of course, i.e. -r rom.bin -i
> boot:boot.bin should store the $regionimage of boot into boot.bin with
> all the layout patches applied. I need to look at the code again to see
> what would happen with this patch applied only.
AFAICS with this patch applied, -r rom.bin -i boot:boot.bin will read
the whole chip, then replace region "boot" with the contents from
boot.bin, then write the resulting image into rom.bin.
More information about the flashrom