[flashrom] Giving to -i option some real use

[Salvador Eduardo Tropea]

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- at 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:
>
> 1) The file is loaded, newcontents is "1230".
> 2) ROM contents are read. oldcontents is "0127"
> 3) 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:
>
> 1) The file is loaded, newcontents is "1230"
> 2) 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