On 16.03.2017 00:08, David Hendricks wrote:
On Fri, Mar 10, 2017 at 8:02 AM, Nico Huber
I've just updated my solution to _the_ layout problem that I wrote last
year . I'm not asking for a review at this moment. There are at least
two competing approaches that I want to discuss first (I couldn't start
a discussion before I wrote it, due to time constraints).
1) walk over the given layout regions in an outer loop and
walk over the erase blocks in each region in an inner loop
This is how my patch tackles it.
2) walk over all erase blocks in an outer loop and
walk over the touched layout regions in an inner loop
Both sounds easy to do but it gets really complicated when you account
for a) regions that are not erase block aligned, b) our fallback to dif-
ferent erase functions (and blocks thereby) and c) read-locks that might
render a) impossible.
I think the second approach results in an extra layer of logic that really
isn't needed. With the first, we:
1. We find what needs to change
2. Select the appropriate block eraser
Until now, I kept flashrom's fallback mechanism: it just tries all erase
functions until one succeeds. I think that's very questionable though.
So far I've only seen it triggered in two scenarios only:
1. Wrong flash chip selected (i.e. wrong opcode or wrong block size).
=> Might be a bug in flashrom that gets never reported.
2. Unreliable reads that made it look like the erase didn't work.
=> Fallback made things even worse, resulting in a full chip erase.
So I'd agree to remove that fallback mechanism.
3. Do the work.
The second approach requires selecting a block erase size beforehand for
the outer loop, and that blockerase size might not be the one we want to
use to actually do the work (especially with the unaligned and optimized
Agreed. In the long run, we'll want to choose the erase function more
One pretty simple solution would be to exclude unaligned layout regions.
How about this? anybody ever needed it?
Historically some ChromeOS devices have had unaligned layout regions.
Generally speaking, excluding them would impose an artificial limitation on
the images that flashrom can handle that will just bite users. Layout
regions might be defined automatically by a firmware's build system without
awareness of a particular flash chip's blockerase sizes, or some users may
want to use unaligned regions to avoid wasting space (like putting multiple
small regions into a 64KB block).
Ok, ok, I'm convinced. Never asked for it ;)
Another related thing is an optimization that I have in mind since ever
I used flashrom: Selection of the biggest possible erase block size.
Currently, if we erase/write say 16 blocks of 4KiB, we waste a lot of
time because most chips would erase a 64KiB block much faster. If we
ever add something like this, I guess it would be easier with approach
1). But maybe it'd be so invasive that it doesn't matter much...
I'd like this as well, and agree that it would be easier with the first
approach. As far as invasiveness goes, the tricky part IMO is updating
portions of read logic to deal with unaligned layout regions to ensure data
outside the targeted region is restored, which requires knowing the
eraseblock size in advance. Otherwise you end up with hacks like this:
I handle this at erase-block level (first code-block in read_erase_
not pretty but reduces the assumptions you have to take care of in