Add flashmap (fmap) support to Flashrom

layout.c

 /*
  * This file is part of the flashrom project.
  *
  * Copyright (C) 2005-2008 coresystems GmbH
  * (Written by Stefan Reinauer <stepan@coresystems.de> for coresystems GmbH)
+ * Portions (C) 2010 Google Inc.
  *
  * This program is free software; you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
  * the Free Software Foundation; version 2 of the License.
  *
  * This program is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  * GNU General Public License for more details.
  *
  * You should have received a copy of the GNU General Public License
  * along with this program; if not, write to the Free Software
  * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA  02110-1301 USA
  */
 
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
 #include <ctype.h>
 #include <limits.h>
 #include "flash.h"
+#include "fmap.h"
 #include "programmer.h"
 
 #if CONFIG_INTERNAL == 1
 char *mainboard_vendor = NULL;
 char *mainboard_part = NULL;
 #endif
 static int romimages = 0;
 
 #define MAX_ROMLAYOUT   32
 
@@ skipping 141 matching lines @@
                              rom_entries[i].start,
                              rom_entries[i].end, rom_entries[i].name);
         }
 
         fclose(romlayout);
 
         return 0;
 }
 #endif
 
+/* returns the number of entries added */
+static int add_fmap_entries(uint8_t *image, int size)
+{
+        int i;
+        struct fmap *fmap;
+        long int fmap_offset;
+
+        fmap_offset = fmap_find(image, size);

[Louis, 2010/12/30 02:57:08]

hm... shall we check the return value for fmap_find()?

[dhendrix, 2010/12/31 00:47:02]

Yes, definitely! I had only tested using BIOSes with valid fmap structures, so I
didn't hit that condition.

+        fmap = (struct fmap *)(image + fmap_offset);;
+
+        for (i = 0; i < fmap->nareas; i++) {
+                rom_entries[romimages].start = fmap->areas[i].offset;
+                rom_entries[romimages].end = fmap->areas[i].offset +
+                                             fmap->areas[i].size;

[Louis, 2010/12/30 02:57:08]

should be .offset + .size "- 1" since .end is the last byte of partition.

[dhendrix, 2010/12/31 00:47:02]

yes

+                if (fmap->areas[i].flags & FMAP_AREA_RO)

[Louis, 2010/12/30 02:57:08]

hm... does this imply we CANNOT update the whole image when factory?

I would suggest we keep .included = 0 at initial. Then set to 1 if user
specifies -i.

[dhendrix, 2010/12/31 00:47:02]

Correct. Using this approach, the way to override the firmware's built-in fmap
would be to use a layout file. So to write the whole ROM, you'd need something
like:

1. echo "0:`flashrom -p internal:bus=spi --get-size` entire_rom" > layout.txt
2. flashrom -p internal:bus=spi -l layout.txt -i entire_rom -w romimage.bin

Your approach has an advantage in the sense that fmap regions will be treated
the same way as layout regions. However, normal firmware updates will require
that the user include each writeable region as a -i option. This is 4 regions on
the current Mario BIOS, and I fear that they may be labeled differently or
changed in future BIOSes.

Both approaches have some significant benefits/disadvantages... Hmmm...

[Louis, 2010/12/31 09:13:30]

This changes the behavior too much and becomes more complicated if I just want
to overwrite a H2C bios. I personally don't like this solution.
Specifying -i is always needed because we only update one pair of {FW, sig} at a
time. The firmware updater never overwrites RW A/B at the same time when AU.
I'd like to suggest keeping original convention. Backward compatible: we don't
have to change existing callers (testing code, updater, ...). We earn the good
experience because we don't have to create layout file when fmap is present.

[dhendrix, 2011/03/04 00:06:17]

Agreed.

+                        rom_entries[romimages].included = 0;
+                else
+                        rom_entries[romimages].included = 1;
+                romimages++;

[Louis, 2010/12/30 02:57:08]

Shall we check the romimages if bigger than size of rom_entries?

[dhendrix, 2010/12/31 00:47:02]

Yes. I added a check near the top of this loop.

btw -- I just realized that we may need more romimage regions, so I sent an
orthogonal CL hoping that we never exceed MAX_ROMLAYOUT:
http://codereview.chromium.org/6001009

+        }
+
+        return fmap->nareas;
+}
+
+
 int find_romentry(char *name)
 {
         int i;
 
         if (!romimages)
                 return -1;
 
         msg_gdbg("Looking for \"%s\"... ", name);
 
         for (i = 0; i < romimages; i++) {
@@ skipping 32 matching lines @@
         }
         return best_entry;
 }
 
 int handle_romentries(struct flashchip *flash, uint8_t *oldcontents, uint8_t *newcontents)
 {
         unsigned int start = 0;
         int entry;
         unsigned int size = flash->total_size * 1024;
 
+        /* add entries for non-readonly regions specified in flashmap */
+        add_fmap_entries(oldcontents, size);
+
         /* If no layout file was specified or the layout file was empty, assume
          * that the user wants to flash the complete new image.
          */
         if (!romimages)
                 return 0;
         /* Non-included romentries are ignored.
          * The union of all included romentries is used from the new image.
          */
         while (start < size) {
                 entry = find_next_included_romentry(start);
                 /* No more romentries for remaining region? */
                 if (entry < 0) {
                         memcpy(newcontents + start, oldcontents + start,
                                size - start);
                         break;
                 }
                 if (rom_entries[entry].start > start)
                         memcpy(newcontents + start, oldcontents + start,
                                rom_entries[entry].start - start);
                 /* Skip to location after current romentry. */
                 start = rom_entries[entry].end + 1;
                 /* Catch overflow. */
                 if (!start)
                         break;
         }
                         
         return 0;
 }