leverage flashrom fast partial read function to speed up VPD read.
Dmitry, this is FYR. After this is committed, the VPD read can be much faster for OOBE to read default locale setting. Hi, David, please kindly review my code. Thanks.
This patch basically re-wrote the flashromRead() and flashromWrite().
In the old code, to handle image on BIOS, we always read the whole flash
chip content and then parse fmap. Since David's CL
(http://codereview.chromium.org/6025013/) makes flashrom to parse fmap,
we can just call flashrom partial read to get VPD partition.
This reduce the VPD read from 5 secs to less than 1 sec.
fmapNormalizeAreaName() is also refined because we no longer generate
layout file for flashrom. Hence, user must specify the exact partition
name on BIOS fmap, and unfortunately Mario BIOS' partition name has space.
Some comments are also refined.
Another minor improvement is that VPD utility now guesses the EPS base address
by looking at eps->table_address. This is more user-friendly when handling the
small VPD partition slice file: no longer specify -E argument. But still can
use -E argument to overwrite it (with -O option).
Change-Id: I7caa215ff775da78b7844586d45e5e1ceae3c7a9
BUG=chromium-os:12355
TEST=Tested on Z600 and Mario machine.
On Z600, I manually run the below commands:
1. a standalone VPD.bin file
2. Mario BIOS with VPD 1.0 partition
3. Mario BIOS with blank VPD partition
4. old version of VPD 2.0 partition (located at 0x100)
5. new BIOS with SPD data present (at 0x400) and VPD 2.0 (at 0x600)
On Mario machine,
% vpd -i "RO VPD" -O
% vpd -i "RO VPD" -s A=B
Review URL: http://codereview.chromium.org/6646016
diff --git a/include/lib/flashrom.h b/include/lib/flashrom.h
index 51d604d..c4268c9 100644
--- a/include/lib/flashrom.h
+++ b/include/lib/flashrom.h
@@ -33,10 +33,10 @@
FLASHROM_FAIL,
};
-int flashromRead(const char* tmp_file);
+int flashromRead(const char* part_file, const char* full_file,
+ const char* partition_name);
-int flashromPartialWrite(const char* tmp_file,
- const off_t section_offset,
- const size_t section_size);
+int flashromPartialWrite(const char* part_file, const char* full_file,
+ const char* partition_name);
#endif /* __LIB_FLASHROM_H__ */
diff --git a/include/lib/fmap.h b/include/lib/fmap.h
index c6678f7..cf0955f 100644
--- a/include/lib/fmap.h
+++ b/include/lib/fmap.h
@@ -81,7 +81,13 @@
/*
* fmapNormalizeAreaName - to protect malicious attack, this function
- * replace all non-digits and non-alphabet chars into underline.
+ * replace not allowed chars into underline. Only following are allowed:
+ *
+ * numbers
+ * digits
+ * space
+ *
+ * The allowing list should increase if fmap area name allows more chars.
*
* @name: point to the name.
*/
diff --git a/include/lib/vpd.h b/include/lib/vpd.h
index 1caaf02..cd18d61 100644
--- a/include/lib/vpd.h
+++ b/include/lib/vpd.h
@@ -33,9 +33,13 @@
/* The default RO VPD location in BIOS image.
* If VPD partition cannot be found on fmap, buildEpsAndTables() will use
* this value to count the offset fields of SPD and VPD 2.0.
- *
- * A user can specify -E argument to change the EPS base value. */
-#define GOOGLE_EPS_BASE 0x000000
+ * User can specify -E argument to change the EPS base value.
+ */
+#define GOOGLE_EPS_BASE 0xFFFFFFFF
+/* 0xFFFFFFFF is a special value, which is the value of a blank flash.
+ * It means we don't know the eps->table_address.
+ * This usually happens when we are creating a new VPD partition without
+ * knowing the flash topology (fmap). */
#define GOOGLE_SPD_OFFSET 0x400
#define GOOGLE_SPD_UUID "75f4926b-9e43-4b32-8979-eb20c0eda76a"
diff --git a/lib/flashrom.c b/lib/flashrom.c
index 0ed4acc..2a3a3f3 100644
--- a/lib/flashrom.c
+++ b/lib/flashrom.c
@@ -34,10 +34,6 @@
* So, define a long enough length here. */
#define CMD_BUF_SIZE (4096)
-#define PARTIAL_WRITE_AREA_NAME "pw_name"
-
-static uint8_t layout_filename[] = "/tmp/vpd.layout.XXXXXX";
-
static uint8_t flashrom_cmd[] = "flashrom";
/* The argument for flashrom.
@@ -46,12 +42,14 @@
*/
static uint8_t flashrom_arguments[] = " -p internal:bus=spi ";
-int flashromRead(const char* tmp_file) {
+int flashromRead(const char* part_file, const char* full_file,
+ const char* partition_name) {
char cmd[CMD_BUF_SIZE];
int ret = 0;
- snprintf(cmd, sizeof(cmd), "%s %s -r '%s' >/dev/null 2>&1",
- flashrom_cmd, flashrom_arguments, tmp_file);
+ snprintf(cmd, sizeof(cmd), "%s %s -i '%s':'%s' -r '%s' >/dev/null 2>&1",
+ flashrom_cmd, flashrom_arguments,
+ partition_name, part_file, full_file);
ret = system(cmd);
if (ret == 0)
return FLASHROM_OK;
@@ -59,36 +57,17 @@
return FLASHROM_FAIL;
}
-int flashromPartialWrite(const char* tmp_file,
- const off_t section_offset,
- const size_t section_size) {
+int flashromPartialWrite(const char* part_file, const char* full_file,
+ const char* partition_name) {
int fd;
char cmd[CMD_BUF_SIZE];
FILE* fp;
int ret = 0;
- fd = mkstemp(layout_filename);
- if (fd < 0) {
- fprintf(stderr, "mkstemp(%s) failed.\n", layout_filename);
- return FLASHROM_FAIL;
- } else {
- close(fd);
- }
-
- /* generate layout file */
- if (!(fp = fopen(layout_filename, "w"))) {
- fprintf(stderr, "cannot create layout file: [%s]\n", layout_filename);
- return FLASHROM_FAIL;
- }
- fprintf(fp, "0x%lx:0x%lx %s\n", section_offset,
- section_offset + section_size - 1,
- PARTIAL_WRITE_AREA_NAME);
- fclose(fp);
-
/* write it back */
- snprintf(cmd, sizeof(cmd), "%s %s -l %s -i '%s' -w '%s' >/tmp/null 2>&1",
- flashrom_cmd, flashrom_arguments, layout_filename,
- PARTIAL_WRITE_AREA_NAME, tmp_file);
+ snprintf(cmd, sizeof(cmd), "%s %s -i '%s':'%s' -w '%s' >/tmp/null 2>&1",
+ flashrom_cmd, flashrom_arguments,
+ partition_name, part_file, full_file);
ret = system(cmd);
if (ret == 0)
diff --git a/lib/fmap.c b/lib/fmap.c
index 26ff6b1..15f0172 100644
--- a/lib/fmap.c
+++ b/lib/fmap.c
@@ -52,7 +52,7 @@
assert(name);
while (*name) {
- if (!(isascii(*name) && isalnum(*name))) {
+ if (!(isascii(*name) && (isalnum(*name) || *name == ' '))) {
*name = '_';
}
name++;
diff --git a/vpd.c b/vpd.c
index 45fbc1d..11787ea 100644
--- a/vpd.c
+++ b/vpd.c
@@ -34,7 +34,12 @@
/* 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";
/* 2 containers:
* file: stores decoded pairs from file.
@@ -61,9 +66,11 @@
* 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(). */
@@ -226,7 +233,7 @@
int retval = 0;
if (!(fp = fopen(filename, "r"))) {
- fprintf(stderr, "[WARN] File [%s] cannot be opened for read. Ignored.\n",
+ fprintf(stderr, "[WARN] File [%s] cannot be opened for read. It's fine.\n",
filename);
return 0;
}
@@ -258,6 +265,7 @@
goto teardown;
}
} else {
+ /* FMAP signature is found, try to search the partition name in table. */
int i;
fmap = (struct fmap *)&read_buf[sig_offset];
@@ -289,10 +297,27 @@
* eps points to the EPS structure, which is usually equal to vpd_buf. */
eps_offset = (uint8_t*)eps - vpd_buf;
+ /* check if ups_base variable needs to be updated. */
+ if (!eps_base_force_specified) {
+ if (eps->table_address > file_size) {
+ /* If the table address field of EPS indicates the position exceeding
+ * the file size, that means some how this field needs to be shifted.
+ * A special case is 0xFFFFFFFF, which means unkowns address. But this
+ * is okay because the later offset of structure table are counted
+ * related to this value. */
+ eps_base = eps->table_address - sizeof(*eps);
+ fprintf(stderr, "[INFO] we guess eps_base is 0x%x.\n", eps_base);
+ }
+ }
+
/* 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. It's fine.\n");
+ retval = 0;
+ goto teardown;
+ } else {
fprintf(stderr, "SMBIOS signature is not matched.\n");
fprintf(stderr, "You may use -O to overwrite the data.\n");
retval = 1;
@@ -315,7 +340,8 @@
/* 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;
@@ -362,7 +388,8 @@
/* 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;
@@ -399,7 +426,7 @@
}
if (header->type != VPD_TYPE_END) {
- fprintf(stderr, "[WARN] we expect the last one is type 127. Ignored.\n");
+ fprintf(stderr, "[WARN] we expect the last one is type 127. It's fine.\n");
}
teardown:
@@ -415,7 +442,7 @@
int eps_len = 0;
int retval = 0;
- memset(eps, 0, sizeof(eps));
+ memset(eps, 0xff, sizeof(eps));
/* encode into buffer */
if (VPD_OK != encodeContainer(&file, max_buf_len, buf, &buf_len)) {
@@ -490,6 +517,20 @@
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");
@@ -551,6 +592,7 @@
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. */
@@ -605,9 +647,14 @@
}
}
- 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;
}
@@ -617,13 +664,14 @@
/* 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)) {
@@ -655,7 +703,8 @@
}
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;
