[flashrom] Layout Aware Flash Reading and Erase Blocks

[Nico Huber]

On 16.03.2017 00:08, David Hendricks wrote:
> On Fri, Mar 10, 2017 at 8:02 AM, Nico Huber <nico.huber at secunet.com> wrote:
>>
>> Hi,
>>
>> I've just updated my solution to _the_ layout problem that I wrote last
>> year [1]. 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).
>>
>> We can
>>
>>   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.
>>
>> or
>>
>>   2) walk over all erase blocks in an outer loop and
>>         walk over the touched layout regions in an inner loop
>>
>> Thoughts?
>>
>> 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
> cases).

Agreed. In the long run, we'll want to choose the erase function more
dynamically.

> 
>>
>> 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:
> https://chromium-review.googlesource.com/c/240176/

I handle this at erase-block level (first code-block in read_erase_
write_block(), https://review.coreboot.org/#/c/17945/2/flashrom.c). It's
not pretty but reduces the assumptions you have to take care of in
higher levels.

Nico