From 5f17458cfbe90171bf6d20d9c36470ebbfb3842f Mon Sep 17 00:00:00 2001 From: Julius Werner Date: Tue, 26 May 2020 19:03:31 -0700 Subject: 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 Change-Id: I55a103180b290c1563e35a25496188b6a82e49ff Reviewed-on: https://review.coreboot.org/c/coreboot/+/41757 Tested-by: build bot (Jenkins) Reviewed-by: Hung-Te Lin --- src/drivers/vpd/Makefile.inc | 9 +- src/drivers/vpd/vpd.c | 204 ++++++++++++++++++++++++++----------------- src/drivers/vpd/vpd.h | 23 ----- src/drivers/vpd/vpd_cbmem.c | 81 ----------------- src/drivers/vpd/vpd_premem.c | 13 --- 5 files changed, 133 insertions(+), 197 deletions(-) delete mode 100644 src/drivers/vpd/vpd_cbmem.c delete mode 100644 src/drivers/vpd/vpd_premem.c (limited to 'src/drivers') 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 #include #include +#include #include #include @@ -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 -#include -#include -#include -#include - -#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 - -#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)); -} -- cgit v1.2.3