leverage flashrom fast partial read function to speed up VPD read.

vpd.c

 /*
  * Copyright (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; either version 2
  * of the License, or (at your option) any later version.
  *
  * This program is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
@@ skipping 16 matching lines @@
 #include "lib/flashrom.h"
 #include "lib/fmap.h"
 #include "lib/lib_vpd.h"
 #include "lib/lib_smbios.h"
 #include "lib/vpd.h"
 #include "lib/vpd_tables.h"
 
 /* The buffer length. Right now the VPD partition size on flash is 128KB. */
 #define BUF_LEN (128 * 1024)
 
-static uint8_t flashrom_tmp_file[] = "/tmp/vpd.flashrom.XXXXXX";
+/* temp filename for partial read area. */
+static uint8_t tmp_part_file[] = "/tmp/vpd.flashrom.XXXXXX";
+/* temp filename for the full flash content (actually only partial content
+ * is available). But for flashrom -w to match flash size, we need to keep this
+ * file from flashromRead() to writeFlashrom(). */
+static uint8_t tmp_full_file[] = "/tmp/vpd.flashrom.XXXXXX";

[dhendrix, 2011/03/10 01:33:51]

Just a thought (not worth the effort for this CL):

As i looked over this file, it seemed there is a lot being done with these
temporary filenames. One example is where the temporary file is made, then the
filename is strdup'd and later passed into another function.

. I am curious if it would simplify the code if users such as loadFile(),
flashromRead(), and flashromPartialWrite() were modified to use file descriptors
instead of file names. That way you could replace these uint8_t arrays with:
#define template    "/tmp/vpd.flashrom.XXXXXX"

then call mkstemp() directly (without myMkTemp).

Anyway, just a thought. The current code should work fine.

[Louis, 2011/03/10 08:22:51]

mkstemp() is a good idea. I will modify that next time I get a chance.

For file descriptor, I am not sure good or bad. Will think more later.

On 2011/03/10 01:33:51, dhendrix wrote:

 
 /* 2 containers:
  *   file:      stores decoded pairs from file.
  *   argument:  stores parsed pairs from command arguments.
  */
 struct PairContainer file;
 struct PairContainer argument;
 
 /* The current padding length value.
  * Default: VPD_AS_LONG_AS
  */
 int pad_value_len = VPD_AS_LONG_AS;
 
 /* The output buffer */
 unsigned char buf[BUF_LEN];
 int buf_len = 0;
 int max_buf_len = sizeof(buf);
 
 /* The EPS base address used to fill the EPS table entry.
  * If the VPD partition can be found in fmap, this points to the starting
  * offset of VPD partition. If not found, this is used to be the base address
  * to increase SPD and VPD 2.0 offset fields.
  *
  * User can overwrite this by -E argument.
  */
 uint32_t eps_base = GOOGLE_EPS_BASE;
+int eps_base_force_specified = 0;  /* a bool value to indicate if -E argument
+                                    * is given. */
 
 /* the fmap name of VPD. */
-uint8_t fmap_vpd_area_name[FMAP_STRLEN] = "RO VPD";
+uint8_t fmap_vpd_area_name[FMAP_STRLEN] = "RO_VPD";
 
 /* If found_vpd, replace the VPD partition when saveFile().
  * If not found, always create new file when saveFlie(). */
 int found_vpd = 0;
 
 /* The VPD partition offset and size in buf[]. The whole partition includes:
  *
  *   SMBIOS EPS
  *   SMBIOS tables[]
  *   SPD
@@ skipping 173 matching lines @@
   if (-1 == sig_offset) {
     /* FMAP signature is not found, assume it is pure VPD partition. */
     vpd_buf = read_buf;
     eps = (struct vpd_entry *)vpd_buf;
 
     /* In overwrite mode, ignore all existing data. */
     if (overwrite_it) {
       retval = 0;
       goto teardown;
     }
+
+    if (!eps_base_force_specified) {
+      /* Since we lost the fmap info in this mode, we try to guess the
+       * eps_base from EPS field. */
+      if (eps->table_address > file_size) {
+        eps_base = eps->table_address - sizeof(*eps);
+        fprintf(stderr, "[INFO] we guess eps_base is 0x%x.\n", eps_base);
+      }
+    }
   } else {
+    /* FMAP signature is found, try to search the partition name in table. */
     int i;
 
     fmap = (struct fmap *)&read_buf[sig_offset];
     for(i = 0; i < fmap->nareas; i++) {
       fmapNormalizeAreaName(fmap->areas[i].name);
     }
 
     if (FMAP_OK == fmapGetArea(fmap_vpd_area_name, fmap,
                                &vpd_offset, &vpd_size)) {
       found_vpd = 1;  /* Mark found here then saveFile() knows where to
@@ skipping 14 matching lines @@
       goto teardown;
     }
   }
   /* Now, vpd_buf points to the VPD partition in buffer.
    * eps points to the EPS structure, which is usually equal to vpd_buf. */
   eps_offset = (uint8_t*)eps - vpd_buf;
 
   /* jump if the VPD partition is not recognized. */
   if (memcmp(VPD_ENTRY_MAGIC, eps, sizeof(VPD_ENTRY_MAGIC) - 1)) {
     /* But OKAY if the VPD partition is all-FF, which is un-used. */
-    if (memcmp("\xff\xff\xff\xff", eps, sizeof(VPD_ENTRY_MAGIC) - 1)) {
+    if (!memcmp("\xff\xff\xff\xff", eps, sizeof(VPD_ENTRY_MAGIC) - 1)) {
+      fprintf(stderr, "[WARN] VPD partition not formatted. Ignored.\n");
+      retval = 0;
+      goto teardown;
+    } else {

[dhendrix, 2011/03/10 01:33:51]

nice!

[Louis, 2011/03/10 08:22:51]

Done.

       fprintf(stderr, "SMBIOS signature is not matched.\n");
       fprintf(stderr, "You may use -O to overwrite the data.\n");
       retval = 1;
       goto teardown;
     }
     /* TODO(yjlou): need more EPS sanity checks here. */
   }
   /* EPS is done above. Parse structure tables below. */
   /* Get the first type 241 blob, at the tail of EPS. */
   header = (struct vpd_header*)(((uint8_t*)eps) + eps->entry_length);
   data = (struct vpd_table_binary_blob_pointer *)
          ((uint8_t *)header + sizeof(*header));
 
   /* TODO(yjlou): Re-factor the parsing code to support more SMBIOS entries.
    *              The current code only supports 2 combinations:
    *                1. Type 241 (SPD) + Type 241 (VPD 2.0) + Type 127
    *                2. Type 241 (VPD 2.0) + Type 127
    */
 
   /* Now header points to the first SMBIOS entry, and data points to the
    * first BBP entry. The first entry could be SPD data. We don't care. */
   if (header->handle != expected_handle) {
-    fprintf(stderr, "The first handle value must be 0, but is %d.\n",
+    fprintf(stderr, "[ERROR] The first handle value must be 0, but is %d.\n"
+                    "        Use -O option to re-format.\n",
                     header->handle);
     retval = 1;
     goto teardown;
   }
   if (header->type != VPD_TYPE_BINARY_BLOB_POINTER) {
     fprintf(stderr, "Expect first entry is type Binary Blob Pointer (241),"
                     " but actually is %d\n", header->type);
     fprintf(stderr, "You may use -O to overwrite the data.\n");
     retval = 1;
     goto teardown;
@@ skipping 26 matching lines @@
       retval = 1;
       goto teardown;
     }
     header = (struct vpd_header*)((uint8_t*)header + table_len);
     data = (struct vpd_table_binary_blob_pointer *)
            ((uint8_t *)header + sizeof(*header));
   }
 
   /* The 2nd could be VPD 2.0 data or End Of Table. */
   if (header->handle != expected_handle) {
-    fprintf(stderr, "The second handle value must be 1, but is %d.\n",
+    fprintf(stderr, "[ERROR] The second handle value must be 1, but is %d.\n"
+                    "        Use -O option to re-format.\n",
                     header->handle);
     retval = 1;
     goto teardown;
   }
   uuid_parse(GOOGLE_VPD_2_0_UUID, vpd_2_0_uuid);
   if (header->type == VPD_TYPE_BINARY_BLOB_POINTER &&
       !memcmp(data->uuid, vpd_2_0_uuid, sizeof(data->uuid))) {
     ++expected_handle;
 
     /* iterate all pairs */
@@ skipping 107 matching lines @@
     fprintf(stderr, "fwrite(VPD 2.0) error (%s)\n", strerror(errno));
     retval = 1;
     goto teardown;
   }
   fclose(fp);
 
 teardown:
   return retval;
 }
 
+int myMkTemp(uint8_t *tmp_file) {
+  int fd;
+  fd = mkstemp(tmp_file);
+  if (fd < 0) {
+    fprintf(stderr, "mkstemp(%s) failed\n", tmp_file);
+  }
+  return fd;
+}
+
+int generateTempFilenames(void) {
+  if (myMkTemp(tmp_part_file) < 0) return -1;
+  if (myMkTemp(tmp_full_file) < 0) return -1;
+  return 0;
+}
 
 static void usage(const char *progname) {
   printf("Chrome OS VPD 2.0 utility --\n");
 #ifdef VPD_VERSION
   printf("%s\n", VPD_VERSION);
 #endif
   printf("\n");
   printf("Usage: %s [OPTION] ...\n", progname);
   printf("   OPTIONs include:\n");
   printf("      -h               This help page and version.\n");
@@ skipping 41 matching lines @@
         goto teardown;
         break;
 
       case 'f':
         filename = strdup(optarg);
         break;
 
       case 'E':
         errno = 0;
         eps_base = strtoul(optarg, (char **) NULL, 0);
+        eps_base_force_specified = 1;
 
         /* FIXME: this is not a stable way to detect error because
          *        implementation may (or may not) assign errno. */
         if (!eps_base && errno == EINVAL) {
           fprintf(stderr, "Not a number for EPS base address: %s\n", optarg);
           retval = 1;
           goto teardown;
         }
         break;
 
@@ skipping 34 matching lines @@
         break;
 
       default:
         fprintf(stderr, "Invalid option, use --help for usage.\n");
         retval = 1;
         goto teardown;
         break;
     }
   }
 
-  fd = mkstemp(flashrom_tmp_file);
-  if (fd < 0) {
-    fprintf(stderr, "mkstemp(%s) failed\n", flashrom_tmp_file);
+  if (optind < argc) {
+    fprintf(stderr, "[ERROR] unexpected argument: %s\n\n", argv[optind]);
+    usage(argv[0]);
     retval = 1;
     goto teardown;
   }
+
+  if (generateTempFilenames() < 0) {
+    retval = 1;
+    goto teardown;
+  }
 
   /* to avoid malicious attack, we replace suspicious chars. */
   fmapNormalizeAreaName(fmap_vpd_area_name);
 
   /* if no filename is specified, call flashrom to read from flash. */
   if (!filename) {
-    if (FLASHROM_OK != flashromRead(flashrom_tmp_file)) {
+    if (FLASHROM_OK != flashromRead(tmp_part_file, tmp_full_file,
+                                    fmap_vpd_area_name)) {
       fprintf(stderr, "flashromRead() error!\n");
       retval = 1;
       goto teardown;
     }
     write_back_to_flash = 1;
-    filename = strdup(flashrom_tmp_file);
+    filename = strdup(tmp_part_file);
   }
 
   if (retval = loadFile(filename, &file, overwrite_it)) {
     fprintf(stderr, "loadFile('%s') error.\n", filename);
     goto teardown;
   }
 
   mergeContainer(&file, &argument);
 
   if (list_it) {
@@ skipping 11 matching lines @@
     printf("%s", list_buf);
   }
 
   if (modified) {
     if (retval = saveFile(&file, filename)) {
       fprintf(stderr, "saveFile('%s') error.\n", filename);
       goto teardown;
     }
 
     if (write_back_to_flash) {
-      if (FLASHROM_OK != flashromPartialWrite(filename, vpd_offset, vpd_size)) {
+      if (FLASHROM_OK != flashromPartialWrite(filename, tmp_full_file,
+                                              fmap_vpd_area_name)) {
         fprintf(stderr, "flashromPartialWrite() error.\n");
         retval = 1;
         goto teardown;
       }
     }
   }
 
 teardown:
   if (spd_data) free(spd_data);
   if (filename) free(filename);
   destroyContainer(&file);
   destroyContainer(&argument);
 
   return retval;
 }