New subject: Giving to -i option some real use

21 Apr 2016

      Hi Michael:
I see your point, and see it can be confusing if the interpretation is 
changed.
What about reading the whole erase regions affected by each interval? 
(like David points out)
I mean this:
/**
  * Look for the starting address of the erase region that contains @addr.
  * The size of this region is returned in @sz
  */
int find_page_start(unsigned int addr, struct eraseblock *blocks, 
unsigned int *sz)
{
     unsigned int page_start, i, size, count, group_size;
     /* Look for the beggining of the erase region containing start 
address */
     page_start = 0;
     for (i = 0; i < NUM_ERASEREGIONS; i++) {
         size = blocks[i].size;
         count = blocks[i].count;
         group_size = size * count;
         if (page_start + group_size > addr) {
             /* The address is inside one of these erase regions */
             *sz = size;
             return page_start + ((addr - page_start) / size) * size;
         } else {/* Not here */
             page_start += group_size;
         }
     }
     msg_gerr("ERROR: address out of memory!?\n");
     exit(1);
     return -1;
}
/**
  * Read all the erase regions containing the @start-@end interval.
  * Returns the last address fetched in @real_end_p
  */
int read_pages(struct flashctx *flash, uint8_t *oldcontents, unsigned 
int start, unsigned int end,
         unsigned int *real_end_p)
{
     int k;
     unsigned int real_start, real_end, size;
k = find_erase_function(flash);
     if (k == -1) {
         msg_gerr("ERROR: No erase function available.\n");
         return -1;
     }
     real_start = find_page_start(start, 
flash->chip->block_erasers[k].eraseblocks, &size);
     msg_gdbg("Start of range 0x%X rounded to page boundary: 0x%X\n", 
start, real_start);
     real_end = find_page_start(end, 
flash->chip->block_erasers[k].eraseblocks, &size);
     real_end += size - 1;
     msg_gdbg("End of range 0x%X rounded to page boundary: 0x%X\n", end, 
real_end);
     *real_end_p = real_end;
     return flash->chip->read(flash, oldcontents + real_start, 
real_start, real_end - real_start + 1);
}
/**
  * Fill the @oldcontents buffer with data from the memory. The regions 
not included by the user are filled
  * with data from @newcontents instead of the actual memory content. It 
forces to skip them.
  * This function is used only when @oldcontents is empty.
  * Is also mandatory that at least one region was specified by the user.
  */
int read_cur_content(struct flashctx *flash, uint8_t *oldcontents, const 
uint8_t *newcontents)
{
     unsigned int start = 0;
     romentry_t *entry;
     unsigned int size = flash->chip->total_size * 1024;
     unsigned int region_end;
     int ret = 0;
/* We will read from memory only the regions indicated by the user.
      * The rest will contain the same as the newcontents */
     memcpy(oldcontents, newcontents, size);
/* Catch a missuse */
     if (num_include_args == 0) {
         msg_perr("Internal error! build_old_image() invoked, but no 
regions specified.\n");
         return 1;
     }
/* Non-included romentries are ignored.
      * The union of all included romentries is fetched from memory.
      */
     while (start < size) {
         entry = get_next_included_romentry(start);
         /* No more romentries for remaining region? */
         if (!entry)
             break;
         /* For included regions, read memory content content. */
         msg_gdbg("Read a chunk 0x%06x-0x%06x.\n", entry->start, 
entry->end);
         ret = read_pages(flash, oldcontents, entry->start, entry->end, 
&region_end);
         if (ret != 0) {
             msg_gerr("Failed to read chunk 0x%06x-0x%06x.\n", 
entry->start, entry->end);
             break;
         }
         start = region_end + 1;
         /* Catch overflow. */
         if (!start)
             break;
     }
     return ret;
}
The find_erase_function() is in flashrom.c and is like this:
/* Look for the first valid erase function. Returns -1 if none.
  */
int find_erase_function(struct flashctx *flash)
{
     int k;
msg_cdbg("\nLooking for an erase function.\n");
     for (k = 0; k < NUM_ERASEFUNCTIONS; k++) {
         if (k != 0)
             msg_cdbg("Looking for another erase function.\n");
         msg_cdbg("Trying erase function %i... ", k);
         if (!check_block_eraser(flash, k, 1)) {
             msg_cdbg("OK\n");
             return k;
         }
     msg_cdbg("INVALID\n");
     }
     msg_cdbg("No usable erase functions left.\n");
     return -1;
}
In the case of the range I'm using I get: (with -VV)
...
Reading old flash chip contents... Read a chunk 0x000000-0x007e2b.
Looking for an erase function.
Trying erase function 0... OK
Start of range 0x0 rounded to page boundary: 0x0
End of range 0x7E2B rounded to page boundary: 0x7FFF
...
In this way we know the content of the blocks that we'll erase. The 
values look ok, since the first eraser have 4k pages.
Note: this code uses the first usable erase function, 
erase_and_write_flash will try the same eraser. If this eraser fails we 
will need more data, but in this case erase_and_write_flash reads all 
the memory again.
Regards, SET
El 20/04/16 a las 15:23, Michael Karcher escribió:
...
On 04/20/2016 12:07 PM, Salvador Eduardo Tropea wrote:
...
/**

Fill the @oldcontents buffer with data from the memory. The regions

not included by the user are filled

with data from @newcontents instead of the actual memory content.

It forces to skip them.

This function is used only when @oldcontents is empty.
Is also mandatory that at least one region was specified by the user.

*/
My intention was to use the same (nasty) tricks that the code already
uses. By forcing areas to match the rest of the code avoids touching it.
My concern is still valid. The claim "It forces to skip them" appears
wrong. If the area of the new and the old image match, the code tries to
optimize by not touching the area, but there is no guarantee that the
area is indeed untouched. If the old contents of the chip has been read,
flashrom "hides" the fact that it touched an area that did not need to
be touched by reflashing the old contents after erasing. This hiding
breaks down if flashrom doesn't know the real contents.
Here is an example of what I mean: Assume a flash chip containing four
nibbles (used instead of bytes for presentation purposes; nibbles are
represented in hex digits); the chip contains two independently erasable
areas of two nibbles each, and I use a layout file like this (the erased
state is the nibble value F).
0:2 please_flash
3:3 please_keep
Assume the flash chip contains 0127, and the new image contains 1230.
Without -A, and selecting the image "please_flash" the following steps
are taken:

The file is loaded, newcontents is "1230".
ROM contents are read. oldcontents is "0127"
build_new_image/modify_new_image_from_old copies the last nibble from

oldcontents to newcontents, because it is in "please_keep", not in
"please_flash". newcontents is now "1237"
4) The first two nibbles are erased, because the old value "01" does not
match "12". Now they are "FF"
5) The first two nibbles are programmed to "12"
6) The second *two* nibbles are erased because "27" does not match "37".
Now they are "FF"
7) The second two nibbles are programmed to "37".
The flash chip now contains 1237.
With -A, the following steps are taken:

The file is loaded, newcontents is "1230"
read_cur_content loads the first three nibbles from the flash memory,

and takes the last one from newcontents. oldcontents is "0120".
3) The first two nibbles are erased, because the old value "01" does not
match "12". Now they are "FF"
4) The first two nibbles are programmed to "12"
5) The second *two* nibbles are erased because "20" does not match "30".
Now they are "FF"
6) The second two nibbles are programmed to "30".
The flash chip now contains 1230.
As you see, the last nibble is modified to "0" instead of being kept at
"7". There might be situations in which it doesn't matter that a nibble
outside of the requested area is changed, but it definitely is different
from how we handled layout files and partial flashing up to now. Note
especially how copying the last nibble (the zero with -A, the 7 without
-A) from newcontents to oldcontents did not force the nibble to be
untouched in either case, but in the first case flashrom knew what to
write so the 7 appears to be unmodified while in fact it was erased and
rewritten.
Regards,
   Michael Karcher
-- 
Ing. Salvador Eduardo Tropea          http://utic.inti.gob.ar/
INTI - Micro y Nanoelectrónica (CMNB) http://www.inti.gob.ar/
Unidad Técnica Sistemas Inteligentes  Av. General Paz 5445
Tel: (+54 11) 4724 6300 ext. 6919     San Martín - B1650KNA
FAX: (+54 11) 4754 5194               Buenos Aires * Argentina

Re: [flashrom] Giving to -i option some real use