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.
a) will always be the case when b) results in the erase chip function.
One pretty simple solution would be to exclude unaligned layout regions. How about this? anybody ever needed 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...
Best regards, Nico
On Fri, Mar 10, 2017 at 8:02 AM, Nico Huber nico.huber@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
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
- 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 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).
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).
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/
-
David Hendricks -
Nico Huber