drivers/vpd: Fix VPD speed regressions on non-x86 devices

CB:34634 expanded the VPD code to also be usable from romstage,
shuffling a few things around and adding some extra infrastructure in
the process. Unfortunately, the changes seem to have only been written
with x86 devices in mind and make coreboot always load the whole VPD
FMAP section (not just the used part) on devices where rdev_mmap() is
not a no-op.

This patch rewrites the VPD code to be based on region_device structures
that only represent the VPD area actually used (rather than the whole
FMAP section), and that only get mapped when accessed. (It would be even
better to pull this concept into the VPD decoder itself, but since that
is taken from third-party code and accesses in early stages aren't very
common, let's not go there for now.) It also moves the copying into
CBMEM to romstage so that late romstage accesses can already benefit
from it, and makes early decoding available in all stages because at
this point, why not.

Also fix a long-standing bug where the 'consumed' counter was not reset
between vpd_decode_string() calls to the RO and the RW VPD.

Signed-off-by: Julius Werner <jwerner@chromium.org>
Change-Id: I55a103180b290c1563e35a25496188b6a82e49ff
Reviewed-on: https://review.coreboot.org/c/coreboot/+/41757
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Hung-Te Lin <hungte@chromium.org>

5 files changed, 133 insertions, 197 deletions

diff --git a/src/drivers/vpd/Makefile.inc b/src/drivers/vpd/Makefile.inc
index cc276e4968..f54c4d0dd9 100644
--- a/src/drivers/vpd/Makefile.inc
+++ b/src/drivers/vpd/Makefile.inc
@@ -1,2 +1,7 @@
-romstage-$(CONFIG_VPD) += vpd_decode.c vpd_premem.c vpd.c
-ramstage-$(CONFIG_VPD) += vpd_decode.c vpd_cbmem.c vpd.c
+# SPDX-License-Identifier: GPL-2.0-only
+
+bootblock-$(CONFIG_VPD) += vpd_decode.c vpd.c
+verstage-$(CONFIG_VPD) += vpd_decode.c vpd.c
+romstage-$(CONFIG_VPD) += vpd_decode.c vpd.c
+postcar-$(CONFIG_VPD) += vpd_decode.c vpd.c
+ramstage-$(CONFIG_VPD) += vpd_decode.c vpd.c
diff --git a/src/drivers/vpd/vpd.c b/src/drivers/vpd/vpd.c
index 7314c35025..f5ac81ebfd 100644
--- a/src/drivers/vpd/vpd.c
+++ b/src/drivers/vpd/vpd.c
@@ -4,6 +4,7 @@
 #include <console/console.h>
 #include <cbmem.h>
 #include <fmap.h>
+#include <program_loading.h>
 #include <string.h>
 #include <timestamp.h>
 
@@ -11,6 +12,23 @@
 #include "vpd_decode.h"
 #include "vpd_tables.h"
 
+/* Currently we only support Google VPD 2.0, which has a fixed offset. */
+enum {
+	CROSVPD_CBMEM_MAGIC = 0x43524f53,
+	CROSVPD_CBMEM_VERSION = 0x0001,
+};
+
+struct vpd_cbmem {
+	uint32_t magic;
+	uint32_t version;
+	uint32_t ro_size;
+	uint32_t rw_size;
+	uint8_t blob[0];
+	/* The blob contains both RO and RW data. It starts with RO (0 ..
+	 * ro_size) and then RW (ro_size .. ro_size+rw_size).
+	 */
+};
+
 struct vpd_gets_arg {
 	const uint8_t *key;
 	const uint8_t *value;
@@ -18,110 +36,137 @@ struct vpd_gets_arg {
 	int matched;
 };
 
-struct vpd_blob vpd_blob;
+static struct region_device ro_vpd, rw_vpd;
 
 /*
- * returns the size of data in a VPD 2.0 formatted fmap region, or 0.
- * Also sets *base as the region's base address.
+ * Initializes a region_device to represent the requested VPD 2.0 formatted
+ * region on flash. On errors rdev->size will be set to 0.
  */
-static int32_t get_vpd_size(const char *fmap_name, int32_t *base)
+static void init_vpd_rdev(const char *fmap_name, struct region_device *rdev)
 {
 	struct google_vpd_info info;
-	struct region_device vpd;
 	int32_t size;
 
-	if (fmap_locate_area_as_rdev(fmap_name, &vpd)) {
+	if (fmap_locate_area_as_rdev(fmap_name, rdev)) {
 		printk(BIOS_ERR, "%s: No %s FMAP section.\n", __func__,
 			fmap_name);
-		return 0;
+		goto fail;
 	}
 
-	size = region_device_sz(&vpd);
+	size = region_device_sz(rdev);
 
 	if ((size < GOOGLE_VPD_2_0_OFFSET + sizeof(info)) ||
-	    rdev_chain(&vpd, &vpd, GOOGLE_VPD_2_0_OFFSET,
+	    rdev_chain(rdev, rdev, GOOGLE_VPD_2_0_OFFSET,
 			size - GOOGLE_VPD_2_0_OFFSET)) {
 		printk(BIOS_ERR, "%s: Too small (%d) for Google VPD 2.0.\n",
 		       __func__, size);
-		return 0;
+		goto fail;
 	}
 
 	/* Try if we can find a google_vpd_info, otherwise read whole VPD. */
-	if (rdev_readat(&vpd, &info, *base, sizeof(info)) != sizeof(info)) {
+	if (rdev_readat(rdev, &info, 0, sizeof(info)) != sizeof(info)) {
 		printk(BIOS_ERR, "ERROR: Failed to read %s header.\n",
 		       fmap_name);
-		return 0;
+		goto fail;
 	}
 
 	if (memcmp(info.header.magic, VPD_INFO_MAGIC, sizeof(info.header.magic))
-	    == 0 && size >= info.size + sizeof(info)) {
-		*base += sizeof(info);
-		size = info.size;
+	    == 0) {
+		if (rdev_chain(rdev, rdev, sizeof(info), info.size)) {
+			printk(BIOS_ERR, "ERROR: %s info size too large.\n",
+			       fmap_name);
+			goto fail;
+		}
 	} else if (info.header.tlv.type == VPD_TYPE_TERMINATOR ||
 		   info.header.tlv.type == VPD_TYPE_IMPLICIT_TERMINATOR) {
 		printk(BIOS_WARNING, "WARNING: %s is uninitialized or empty.\n",
 		       fmap_name);
-		size = 0;
-	} else {
-		size -= GOOGLE_VPD_2_0_OFFSET;
+		goto fail;
+	}
+
+	return;
+
+fail:
+	memset(rdev, 0, sizeof(*rdev));
+}
+
+static int init_vpd_rdevs_from_cbmem(void)
+{
+	if (!cbmem_possibly_online())
+		return -1;
+
+	struct vpd_cbmem *cbmem = cbmem_find(CBMEM_ID_VPD);
+	if (!cbmem)
+		return -1;
+
+	rdev_chain(&ro_vpd, &addrspace_32bit.rdev,
+		   (uintptr_t)cbmem->blob, cbmem->ro_size);
+	rdev_chain(&rw_vpd, &addrspace_32bit.rdev,
+		   (uintptr_t)cbmem->blob + cbmem->ro_size, cbmem->rw_size);
+
+	return 0;
+}
+
+static void init_vpd_rdevs(void)
+{
+	static bool done = false;
+
+	if (done)
+		return;
+
+	if (init_vpd_rdevs_from_cbmem() != 0) {
+		init_vpd_rdev("RO_VPD", &ro_vpd);
+		init_vpd_rdev("RW_VPD", &rw_vpd);
 	}
 
-	return size;
+	done = true;
 }
 
-static void vpd_get_blob(void)
+static void cbmem_add_cros_vpd(int is_recovery)
 {
-	int32_t ro_vpd_base = 0;
-	int32_t rw_vpd_base = 0;
-	int32_t ro_vpd_size = get_vpd_size("RO_VPD", &ro_vpd_base);
-	int32_t rw_vpd_size = get_vpd_size("RW_VPD", &rw_vpd_base);
+	struct vpd_cbmem *cbmem;
+
+	timestamp_add_now(TS_START_COPYVPD);
+
+	init_vpd_rdevs();
 
 	/* Return if no VPD at all */
-	if (ro_vpd_size == 0 && rw_vpd_size == 0)
+	if (region_device_sz(&ro_vpd) == 0 && region_device_sz(&rw_vpd) == 0)
 		return;
 
-	vpd_blob.ro_base = NULL;
-	vpd_blob.ro_size = 0;
-	vpd_blob.rw_base = NULL;
-	vpd_blob.rw_size = 0;
+	size_t ro_size = region_device_sz(&ro_vpd);
+	size_t rw_size = region_device_sz(&rw_vpd);
 
-	struct region_device vpd;
+	cbmem = cbmem_add(CBMEM_ID_VPD, sizeof(*cbmem) + ro_size + rw_size);
+	if (!cbmem) {
+		printk(BIOS_ERR, "%s: Failed to allocate CBMEM (%zu+%zu).\n",
+			__func__, ro_size, rw_size);
+		return;
+	}
+
+	cbmem->magic = CROSVPD_CBMEM_MAGIC;
+	cbmem->version = CROSVPD_CBMEM_VERSION;
+	cbmem->ro_size = ro_size;
+	cbmem->rw_size = rw_size;
 
-	if (ro_vpd_size) {
-		if (fmap_locate_area_as_rdev("RO_VPD", &vpd)) {
-			/* shouldn't happen, but let's be extra defensive */
-			printk(BIOS_ERR, "%s: No RO_VPD FMAP section.\n",
-				__func__);
-			return;
+	if (ro_size) {
+		if (rdev_readat(&ro_vpd, cbmem->blob, 0, ro_size) != ro_size) {
+			printk(BIOS_ERR, "ERROR: Couldn't read RO VPD\n");
+			cbmem->ro_size = ro_size = 0;
 		}
-		rdev_chain(&vpd, &vpd, GOOGLE_VPD_2_0_OFFSET,
-			region_device_sz(&vpd) - GOOGLE_VPD_2_0_OFFSET);
-		vpd_blob.ro_base = (uint8_t *)(rdev_mmap_full(&vpd) +
-			sizeof(struct google_vpd_info));
-		vpd_blob.ro_size = ro_vpd_size;
+		timestamp_add_now(TS_END_COPYVPD_RO);
 	}
-	if (rw_vpd_size) {
-		if (fmap_locate_area_as_rdev("RW_VPD", &vpd)) {
-			/* shouldn't happen, but let's be extra defensive */
-			printk(BIOS_ERR, "%s: No RW_VPD FMAP section.\n",
-				__func__);
-			return;
+
+	if (rw_size) {
+		if (rdev_readat(&rw_vpd, cbmem->blob + ro_size, 0, rw_size)
+								 != rw_size) {
+			printk(BIOS_ERR, "ERROR: Couldn't read RW VPD\n");
+			cbmem->rw_size = rw_size = 0;
 		}
-		rdev_chain(&vpd, &vpd, GOOGLE_VPD_2_0_OFFSET,
-			region_device_sz(&vpd) - GOOGLE_VPD_2_0_OFFSET);
-		vpd_blob.rw_base = (uint8_t *)(rdev_mmap_full(&vpd) +
-			sizeof(struct google_vpd_info));
-		vpd_blob.rw_size = rw_vpd_size;
+		timestamp_add_now(TS_END_COPYVPD_RW);
 	}
-	vpd_blob.initialized = true;
-}
 
-const struct vpd_blob *vpd_load_blob(void)
-{
-	if (vpd_blob.initialized == false)
-		vpd_get_blob();
-
-	return &vpd_blob;
+	init_vpd_rdevs_from_cbmem();
 }
 
 static int vpd_gets_callback(const uint8_t *key, uint32_t key_len,
@@ -141,33 +186,34 @@ static int vpd_gets_callback(const uint8_t *key, uint32_t key_len,
 	return VPD_DECODE_FAIL;
 }
 
-const void *vpd_find(const char *key, int *size, enum vpd_region region)
+static void vpd_find_in(struct region_device *rdev, struct vpd_gets_arg *arg)
 {
-	struct vpd_blob blob = {0};
+	if (region_device_sz(rdev) == 0)
+		return;
 
-	vpd_get_buffers(&blob);
-	if (blob.ro_size == 0 && blob.rw_size == 0)
-		return NULL;
+	uint32_t consumed = 0;
+	void *mapping = rdev_mmap_full(rdev);
+	while (vpd_decode_string(region_device_sz(rdev), mapping,
+		&consumed, vpd_gets_callback, arg) == VPD_DECODE_OK) {
+	/* Iterate until found or no more entries. */
+	}
+	rdev_munmap(rdev, mapping);
+}
 
+const void *vpd_find(const char *key, int *size, enum vpd_region region)
+{
 	struct vpd_gets_arg arg = {0};
-	uint32_t consumed = 0;
 
 	arg.key = (const uint8_t *)key;
 	arg.key_len = strlen(key);
 
-	if ((region == VPD_ANY || region == VPD_RO) && blob.ro_size != 0) {
-		while (vpd_decode_string(blob.ro_size, blob.ro_base,
-			&consumed, vpd_gets_callback, &arg) == VPD_DECODE_OK) {
-		/* Iterate until found or no more entries. */
-		}
-	}
+	init_vpd_rdevs();
 
-	if ((!arg.matched && region != VPD_RO) && blob.rw_size != 0) {
-		while (vpd_decode_string(blob.rw_size, blob.rw_base,
-			&consumed, vpd_gets_callback, &arg) == VPD_DECODE_OK) {
-		/* Iterate until found or no more entries. */
-		}
-	}
+	if (region != VPD_RW)
+		vpd_find_in(&ro_vpd, &arg);
+
+	if (!arg.matched && region != VPD_RO)
+		vpd_find_in(&rw_vpd, &arg);
 
 	if (!arg.matched)
 		return NULL;
@@ -223,3 +269,5 @@ bool vpd_get_bool(const char *key, enum vpd_region region, uint8_t *val)
 	} else
 		return false;
 }
+
+ROMSTAGE_CBMEM_INIT_HOOK(cbmem_add_cros_vpd)
diff --git a/src/drivers/vpd/vpd.h b/src/drivers/vpd/vpd.h
index a78662579a..d54cef8df1 100644
--- a/src/drivers/vpd/vpd.h
+++ b/src/drivers/vpd/vpd.h
@@ -13,29 +13,6 @@ enum vpd_region {
 	VPD_RW = 2
 };
 
-/* VPD 2.0 data blob structure */
-struct vpd_blob {
-	bool initialized;
-	uint8_t  *ro_base;
-	uint32_t ro_size;
-	uint8_t  *rw_base;
-	uint32_t rw_size;
-};
-extern struct vpd_blob g_vpd_blob;
-
-/*
- * This function loads g_vpd_blob global variable.
- * The variable is initialized if it was not.
- */
-const struct vpd_blob *vpd_load_blob(void);
-
-/*
- * This function gets the base address and size of
- * buffers for RO_VPD/RW_VPD binary blobs, and sets
- * the struct.
- */
-void vpd_get_buffers(struct vpd_blob *blob);
-
 /*
  * Reads VPD string value by key.
  *
diff --git a/src/drivers/vpd/vpd_cbmem.c b/src/drivers/vpd/vpd_cbmem.c
deleted file mode 100644
index eadda525d6..0000000000
--- a/src/drivers/vpd/vpd_cbmem.c
+++ /dev/null
@@ -1,81 +0,0 @@
-/* SPDX-License-Identifier: BSD-3-Clause */
-
-#include <console/console.h>
-#include <cbmem.h>
-#include <fmap.h>
-#include <string.h>
-#include <timestamp.h>
-
-#include "vpd_tables.h"
-#include "vpd.h"
-
-/* Currently we only support Google VPD 2.0, which has a fixed offset. */
-enum {
-	CROSVPD_CBMEM_MAGIC = 0x43524f53,
-	CROSVPD_CBMEM_VERSION = 0x0001,
-};
-
-struct vpd_cbmem {
-	uint32_t magic;
-	uint32_t version;
-	uint32_t ro_size;
-	uint32_t rw_size;
-	uint8_t blob[0];
-	/* The blob contains both RO and RW data. It starts with RO (0 ..
-	 * ro_size) and then RW (ro_size .. ro_size+rw_size).
-	 */
-};
-
-static void cbmem_add_cros_vpd(int is_recovery)
-{
-	struct vpd_cbmem *cbmem;
-	const struct vpd_blob *blob;
-
-	timestamp_add_now(TS_START_COPYVPD);
-
-	blob = vpd_load_blob();
-
-	/* Return if no VPD at all */
-	if (blob->ro_size == 0 && blob->rw_size == 0)
-		return;
-
-	cbmem = cbmem_add(CBMEM_ID_VPD, sizeof(*cbmem) + blob->ro_size +
-		blob->rw_size);
-	if (!cbmem) {
-		printk(BIOS_ERR, "%s: Failed to allocate CBMEM (%u+%u).\n",
-			__func__, blob->ro_size, blob->rw_size);
-		return;
-	}
-
-	cbmem->magic = CROSVPD_CBMEM_MAGIC;
-	cbmem->version = CROSVPD_CBMEM_VERSION;
-	cbmem->ro_size = blob->ro_size;
-	cbmem->rw_size = blob->rw_size;
-
-	if (blob->ro_size) {
-		memcpy(cbmem->blob, blob->ro_base, blob->ro_size);
-		timestamp_add_now(TS_END_COPYVPD_RO);
-	}
-
-	if (blob->rw_size) {
-		memcpy(cbmem->blob + blob->ro_size, blob->rw_base,
-			blob->rw_size);
-		timestamp_add_now(TS_END_COPYVPD_RW);
-	}
-}
-
-void vpd_get_buffers(struct vpd_blob *blob)
-{
-	const struct vpd_cbmem *vpd;
-
-	vpd = cbmem_find(CBMEM_ID_VPD);
-	if (!vpd || !vpd->ro_size)
-		return;
-
-	blob->ro_base = (void *)vpd->blob;
-	blob->ro_size = vpd->ro_size;
-	blob->rw_base = (void *)vpd->blob + vpd->ro_size;
-	blob->rw_size = vpd->rw_size;
-}
-
-RAMSTAGE_CBMEM_INIT_HOOK(cbmem_add_cros_vpd)
diff --git a/src/drivers/vpd/vpd_premem.c b/src/drivers/vpd/vpd_premem.c
deleted file mode 100644
index 641bf898df..0000000000
--- a/src/drivers/vpd/vpd_premem.c
+++ /dev/null
@@ -1,13 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0-only */
-
-#include <string.h>
-
-#include "vpd.h"
-
-void vpd_get_buffers(struct vpd_blob *blob)
-{
-	const struct vpd_blob *b;
-
-	b = vpd_load_blob();
-	memcpy(blob, b, sizeof(*b));
-}