Merge patch series "vbe: Series part F"

Simon Glass <[email protected]> says:

This includes various patches towards implementing the VBE abrec
bootmeth in U-Boot. It mostly focuses on introducing a relocating
SPL-loader so that VBE can run in the limited amount of SRAM available
on many devices.

Another minor new feature is support in VBE for specifying the image
phase when loading from a FIT. This allows a single FIT to include
images for several boot phases, thus simplifying image-creation.

One lingering niggle in this series is that it has a different code path
for sandbox, since it does not support the relocating jump. It should be
possible to resolve this with additional work, but I have not attempted
this so far.

For v2, I have split the first patch into 5 pieces, to make it easier to
see the code-size impact, plus added a few tweaks to reduce code size.

Again, only MMC is supported so far.

Looking ahead, series G will have some more plumbing and H some rk3399
pieces. That should be enough to complete these feature.

Here is a run in my lab, with the VBE ABrec bootmeth. You can see that
VPL runs before memory is set up. SPL sets up memory and can be upgraded
in the field reliably.

$ ub-int vbe
Building U-Boot in sourcedir for rk3399-generic
Bootstrapping U-Boot from dir /tmp/b/rk3399-generic
Writing U-Boot using method rockchip

U-Boot TPL 2025.01-rc3-00345-gdfbdbf1eb56c-dirty (Jan 08 2025 - 10:47:58)
Trying to boot from vbe_abrec
load: Firefly-RK3399 Board
   Using 'config-3' configuration
   Trying 'image-vpl' firmware subimage
   Using 'config-3' configuration
   Trying 'fdt-3' fdt subimage

U-Boot VPL 2025.01-rc3-00345-gdfbdbf1eb56c-dirty (Jan 08 2025 - 10:47:58)
Trying to boot from vbe_abrec
load: Firefly-RK3399 Board
Starting with empty state
VBE: Firmware pick A at 800000
   Using 'config-3' configuration
   Trying 'spl' firmware subimage
   Using 'config-3' configuration
   Trying 'fdt-3' fdt subimage
Channel 0: DDR3, 800MHz
BW=32 Col=10 Bk=8 CS0 Row=15 CS1 Row=15 CS=2 Die BW=16 Size=2048MB
Channel 1: DDR3, 800MHz
BW=32 Col=10 Bk=8 CS0 Row=15 CS1 Row=15 CS=2 Die BW=16 Size=2048MB
256B stride

U-Boot SPL 2025.01-rc3-00345-gdfbdbf1eb56c-dirty (Jan 08 2025 - 10:47:58 -0700)
Trying to boot from vbe_abrec
load: Firefly-RK3399 Board
VBE: Firmware pick A at 900000
load_simple_fit: Skip load 'atf-5': image size is 0!
Relocating bloblist ff8eff00 to 100000: done
ns16550_serial serial@ff1a0000: pinctrl_select_state_full: uclass_get_device_by_phandle_id: err=-19

U-Boot 2025.01-rc3-00345-gdfbdbf1eb56c-dirty (Jan 08 2025 - 10:47:58 -0700)

SoC: Rockchip rk3399
Reset cause: POR
Model: Firefly-RK3399 Board
DRAM:  4 GiB (effective 3.9 GiB)
Core:  314 devices, 33 uclasses, devicetree: separate
MMC:   mmc@fe310000: 3, mmc@fe320000: 1, mmc@fe330000: 0
Loading Environment from SPIFlash... Invalid bus 0 (err=-19)
*** Warning - spi_flash_probe_bus_cs() failed, using default environment

In:    serial,usbkbd
Out:   serial,vidconsole
Err:   serial,vidconsole
Model: Firefly-RK3399 Board
Net:   PMIC:  RK808
eth0: ethernet@fe300000

starting USB...
Bus usb@fe380000: USB EHCI 1.00
Bus usb@fe3a0000: USB OHCI 1.0
Bus usb@fe3c0000: USB EHCI 1.00
Bus usb@fe3e0000: USB OHCI 1.0
Bus usb@fe900000: Register 2000140 NbrPorts 2
Starting the controller
USB XHCI 1.10
scanning bus usb@fe380000 for devices... 1 USB Device(s) found
scanning bus usb@fe3a0000 for devices... 1 USB Device(s) found
scanning bus usb@fe3c0000 for devices... 2 USB Device(s) found
scanning bus usb@fe3e0000 for devices... 1 USB Device(s) found
scanning bus usb@fe900000 for devices... 1 USB Device(s) found
       scanning usb for storage devices... 0 Storage Device(s) found
Hit any key to stop autoboot:  0

Link: https://lore.kernel.org/r/[email protected]

6 files changed, 614 insertions, 221 deletions

diff --git a/boot/Makefile b/boot/Makefile
index a24fd90c510..c2753de8163 100644
--- a/boot/Makefile
+++ b/boot/Makefile
@@ -65,7 +65,7 @@ endif
 
 obj-$(CONFIG_$(PHASE_)BOOTMETH_VBE) += vbe.o
 obj-$(CONFIG_$(PHASE_)BOOTMETH_VBE_REQUEST) += vbe_request.o
-obj-$(CONFIG_$(PHASE_)BOOTMETH_VBE_SIMPLE) += vbe_simple.o
+obj-$(CONFIG_$(PHASE_)BOOTMETH_VBE_SIMPLE) += vbe_simple.o vbe_common.o
 obj-$(CONFIG_$(PHASE_)BOOTMETH_VBE_SIMPLE_FW) += vbe_simple_fw.o
 obj-$(CONFIG_$(PHASE_)BOOTMETH_VBE_SIMPLE_OS) += vbe_simple_os.o
 
diff --git a/boot/vbe_common.c b/boot/vbe_common.c
new file mode 100644
index 00000000000..0d51fe762c3
--- /dev/null
+++ b/boot/vbe_common.c
@@ -0,0 +1,375 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Verified Boot for Embedded (VBE) common functions
+ *
+ * Copyright 2024 Google LLC
+ * Written by Simon Glass <[email protected]>
+ */
+
+#include <bootstage.h>
+#include <dm.h>
+#include <blk.h>
+#include <image.h>
+#include <mapmem.h>
+#include <memalign.h>
+#include <spl.h>
+#include <u-boot/crc.h>
+#include "vbe_common.h"
+
+binman_sym_declare(ulong, u_boot_vpl_nodtb, size);
+binman_sym_declare(ulong, u_boot_vpl_bss_pad, size);
+binman_sym_declare(ulong, u_boot_spl_nodtb, size);
+binman_sym_declare(ulong, u_boot_spl_bss_pad, size);
+
+int vbe_get_blk(const char *storage, struct udevice **blkp)
+{
+	struct blk_desc *desc;
+	char devname[16];
+	const char *end;
+	int devnum;
+
+	/* First figure out the block device */
+	log_debug("storage=%s\n", storage);
+	devnum = trailing_strtoln_end(storage, NULL, &end);
+	if (devnum == -1)
+		return log_msg_ret("num", -ENODEV);
+	if (end - storage >= sizeof(devname))
+		return log_msg_ret("end", -E2BIG);
+	strlcpy(devname, storage, end - storage + 1);
+	log_debug("dev=%s, %x\n", devname, devnum);
+
+	desc = blk_get_dev(devname, devnum);
+	if (!desc)
+		return log_msg_ret("get", -ENXIO);
+	*blkp = desc->bdev;
+
+	return 0;
+}
+
+int vbe_read_version(struct udevice *blk, ulong offset, char *version,
+		     int max_size)
+{
+	ALLOC_CACHE_ALIGN_BUFFER(u8, buf, MMC_MAX_BLOCK_LEN);
+
+	/* we can use an assert() here since we already read only one block */
+	assert(max_size <= MMC_MAX_BLOCK_LEN);
+
+	/*
+	 * we can use an assert() here since reading the wrong block will just
+	 * cause an invalid version-string to be (safely) read
+	 */
+	assert(!(offset & (MMC_MAX_BLOCK_LEN - 1)));
+
+	offset /= MMC_MAX_BLOCK_LEN;
+
+	if (blk_read(blk, offset, 1, buf) != 1)
+		return log_msg_ret("read", -EIO);
+	strlcpy(version, buf, max_size);
+	log_debug("version=%s\n", version);
+
+	return 0;
+}
+
+int vbe_read_nvdata(struct udevice *blk, ulong offset, ulong size, u8 *buf)
+{
+	uint hdr_ver, hdr_size, data_size, crc;
+	const struct vbe_nvdata *nvd;
+
+	/* we can use an assert() here since we already read only one block */
+	assert(size <= MMC_MAX_BLOCK_LEN);
+
+	/*
+	 * We can use an assert() here since reading the wrong block will just
+	 * cause invalid state to be (safely) read. If the crc passes, then we
+	 * obtain invalid state and it will likely cause booting to fail.
+	 *
+	 * VBE relies on valid values being in U-Boot's devicetree, so this
+	 * should not every be wrong on a production device.
+	 */
+	assert(!(offset & (MMC_MAX_BLOCK_LEN - 1)));
+
+	if (offset & (MMC_MAX_BLOCK_LEN - 1))
+		return log_msg_ret("get", -EBADF);
+	offset /= MMC_MAX_BLOCK_LEN;
+
+	if (blk_read(blk, offset, 1, buf) != 1)
+		return log_msg_ret("read", -EIO);
+	nvd = (struct vbe_nvdata *)buf;
+	hdr_ver = (nvd->hdr & NVD_HDR_VER_MASK) >> NVD_HDR_VER_SHIFT;
+	hdr_size = (nvd->hdr & NVD_HDR_SIZE_MASK) >> NVD_HDR_SIZE_SHIFT;
+	if (hdr_ver != NVD_HDR_VER_CUR)
+		return log_msg_ret("hdr", -EPERM);
+	data_size = 1 << hdr_size;
+	if (!data_size || data_size > sizeof(*nvd))
+		return log_msg_ret("sz", -EPERM);
+
+	crc = crc8(0, buf + 1, data_size - 1);
+	if (crc != nvd->crc8)
+		return log_msg_ret("crc", -EPERM);
+
+	return 0;
+}
+
+/**
+ * h_vbe_load_read() - Handler for reading an SPL image from a FIT
+ *
+ * See spl_load_reader for the definition
+ */
+ulong h_vbe_load_read(struct spl_load_info *load, ulong off, ulong size,
+		      void *buf)
+{
+	struct blk_desc *desc = load->priv;
+	lbaint_t sector = off >> desc->log2blksz;
+	lbaint_t count = size >> desc->log2blksz;
+	int ret;
+
+	log_debug("vbe read log2blksz %x offset %lx sector %lx count %lx\n",
+		  desc->log2blksz, (ulong)off, (long)sector, (ulong)count);
+
+	ret = blk_dread(desc, sector, count, buf);
+	log_debug("ret=%x\n", ret);
+	if (ret < 0)
+		return ret;
+
+	return ret << desc->log2blksz;
+}
+
+int vbe_read_fit(struct udevice *blk, ulong area_offset, ulong area_size,
+		 struct spl_image_info *image, ulong *load_addrp, ulong *lenp,
+		 char **namep)
+{
+	ALLOC_CACHE_ALIGN_BUFFER(u8, sbuf, MMC_MAX_BLOCK_LEN);
+	ulong size, blknum, addr, len, load_addr, num_blks, spl_load_addr;
+	ulong aligned_size, fdt_load_addr, fdt_size;
+	const char *fit_uname, *fit_uname_config;
+	struct bootm_headers images = {};
+	enum image_phase_t phase;
+	struct blk_desc *desc;
+	int node, ret;
+	bool for_xpl;
+	void *buf;
+
+	desc = dev_get_uclass_plat(blk);
+
+	/* read in one block to find the FIT size */
+	blknum =  area_offset / desc->blksz;
+	log_debug("read at %lx, blknum %lx\n", area_offset, blknum);
+	ret = blk_read(blk, blknum, 1, sbuf);
+	if (ret < 0)
+		return log_msg_ret("rd", ret);
+	else if (ret != 1)
+		return log_msg_ret("rd2", -EIO);
+
+	ret = fdt_check_header(sbuf);
+	if (ret < 0)
+		return log_msg_ret("fdt", -EINVAL);
+	size = fdt_totalsize(sbuf);
+	if (size > area_size)
+		return log_msg_ret("fdt", -E2BIG);
+	log_debug("FIT size %lx\n", size);
+	aligned_size = ALIGN(size, desc->blksz);
+
+	/*
+	 * Load the FIT into the SPL memory. This is typically a FIT with
+	 * external data, so this is quite small, perhaps a few KB.
+	 */
+	if (IS_ENABLED(CONFIG_SANDBOX)) {
+		addr = CONFIG_VAL(TEXT_BASE);
+		buf = map_sysmem(addr, size);
+	} else {
+		buf = malloc(aligned_size);
+		if (!buf)
+			return log_msg_ret("fit", -ENOMEM);
+		addr = map_to_sysmem(buf);
+	}
+	num_blks = aligned_size / desc->blksz;
+	log_debug("read %lx, %lx blocks to %lx / %p\n", aligned_size, num_blks,
+		  addr, buf);
+	ret = blk_read(blk, blknum, num_blks, buf);
+	if (ret < 0)
+		return log_msg_ret("rd3", ret);
+	else if (ret != num_blks)
+		return log_msg_ret("rd4", -EIO);
+	log_debug("check total size %x off_dt_strings %x\n", fdt_totalsize(buf),
+		  fdt_off_dt_strings(buf));
+
+#if CONFIG_IS_ENABLED(SYS_MALLOC_F)
+	log_debug("malloc base %lx ptr %x limit %x top %lx\n",
+		  gd->malloc_base, gd->malloc_ptr, gd->malloc_limit,
+		  gd->malloc_base + gd->malloc_limit);
+#endif
+	/* figure out the phase to load */
+	phase = IS_ENABLED(CONFIG_TPL_BUILD) ? IH_PHASE_NONE :
+		IS_ENABLED(CONFIG_VPL_BUILD) ? IH_PHASE_SPL : IH_PHASE_U_BOOT;
+
+	/*
+	 * Load the image from the FIT. We ignore any load-address information
+	 * so in practice this simply locates the image in the external-data
+	 * region and returns its address and size. Since we only loaded the FIT
+	 * itself, only a part of the image will be present, at best.
+	 */
+	fit_uname = NULL;
+	fit_uname_config = NULL;
+	log_debug("loading FIT\n");
+
+	if (xpl_phase() == PHASE_SPL && !IS_ENABLED(CONFIG_SANDBOX)) {
+		struct spl_load_info info;
+
+		spl_load_init(&info, h_vbe_load_read, desc, desc->blksz);
+		xpl_set_phase(&info, IH_PHASE_U_BOOT);
+		log_debug("doing SPL from %s blksz %lx log2blksz %x area_offset %lx + fdt_size %lx\n",
+			  blk->name, desc->blksz, desc->log2blksz, area_offset, ALIGN(size, 4));
+		ret = spl_load_simple_fit(image, &info, area_offset, buf);
+		log_debug("spl_load_abrec_fit() ret=%d\n", ret);
+
+		return ret;
+	}
+
+	ret = fit_image_load(&images, addr, &fit_uname, &fit_uname_config,
+			     IH_ARCH_DEFAULT, image_ph(phase, IH_TYPE_FIRMWARE),
+			     BOOTSTAGE_ID_FIT_SPL_START, FIT_LOAD_IGNORED,
+			     &load_addr, &len);
+	if (ret == -ENOENT) {
+		ret = fit_image_load(&images, addr, &fit_uname,
+				     &fit_uname_config, IH_ARCH_DEFAULT,
+				     image_ph(phase, IH_TYPE_LOADABLE),
+				     BOOTSTAGE_ID_FIT_SPL_START,
+				     FIT_LOAD_IGNORED, &load_addr, &len);
+	}
+	if (ret < 0)
+		return log_msg_ret("ld", ret);
+	node = ret;
+	log_debug("load %lx size %lx\n", load_addr, len);
+
+	fdt_load_addr = 0;
+	fdt_size = 0;
+	if ((xpl_phase() == PHASE_TPL || xpl_phase() == PHASE_VPL) &&
+	    !IS_ENABLED(CONFIG_SANDBOX)) {
+		/* allow use of a different image from the configuration node */
+		fit_uname = NULL;
+		ret = fit_image_load(&images, addr, &fit_uname,
+				     &fit_uname_config, IH_ARCH_DEFAULT,
+				     image_ph(phase, IH_TYPE_FLATDT),
+				     BOOTSTAGE_ID_FIT_SPL_START,
+				     FIT_LOAD_IGNORED, &fdt_load_addr,
+				     &fdt_size);
+		fdt_size = ALIGN(fdt_size, desc->blksz);
+		log_debug("FDT noload to %lx size %lx\n", fdt_load_addr,
+			  fdt_size);
+	}
+
+	for_xpl = !USE_BOOTMETH && CONFIG_IS_ENABLED(RELOC_LOADER);
+	if (for_xpl) {
+		image->size = len;
+		image->fdt_size = fdt_size;
+		ret = spl_reloc_prepare(image, &spl_load_addr);
+		if (ret)
+			return log_msg_ret("spl", ret);
+	}
+	if (!IS_ENABLED(CONFIG_SANDBOX))
+		image->os = IH_OS_U_BOOT;
+
+	/* For FIT external data, read in the external data */
+	log_debug("load_addr %lx len %lx addr %lx aligned_size %lx\n",
+		  load_addr, len, addr, aligned_size);
+	if (load_addr + len > addr + aligned_size) {
+		ulong base, full_size, offset, extra, fdt_base, fdt_full_size;
+		ulong fdt_offset;
+		void *base_buf, *fdt_base_buf;
+
+		/* Find the start address to load from */
+		base = ALIGN_DOWN(load_addr, desc->blksz);
+
+		offset = area_offset + load_addr - addr;
+		blknum = offset / desc->blksz;
+		extra = offset % desc->blksz;
+
+		/*
+		 * Get the total number of bytes to load, taking care of
+		 * block alignment
+		 */
+		full_size = len + extra;
+
+		/*
+		 * Get the start block number, number of blocks and the address
+		 * to load to, then load the blocks
+		 */
+		num_blks = DIV_ROUND_UP(full_size, desc->blksz);
+		if (for_xpl)
+			base = spl_load_addr;
+		base_buf = map_sysmem(base, full_size);
+		ret = blk_read(blk, blknum, num_blks, base_buf);
+		log_debug("read foffset %lx blknum %lx full_size %lx num_blks %lx to %lx / %p: ret=%d\n",
+			  offset - 0x8000, blknum, full_size, num_blks, base, base_buf,
+			  ret);
+		if (ret < 0)
+			return log_msg_ret("rd", ret);
+		if (ret != num_blks)
+			return log_msg_ret("rd", -EIO);
+		if (extra && !IS_ENABLED(CONFIG_SANDBOX)) {
+			log_debug("move %p %p %lx\n", base_buf,
+				  base_buf + extra, len);
+			memmove(base_buf, base_buf + extra, len);
+		}
+
+		if ((xpl_phase() == PHASE_VPL || xpl_phase() == PHASE_TPL) &&
+		    !IS_ENABLED(CONFIG_SANDBOX)) {
+			image->load_addr = spl_get_image_text_base();
+			image->entry_point = image->load_addr;
+		}
+
+		/* now the FDT */
+		if (fdt_size) {
+			fdt_offset = area_offset + fdt_load_addr - addr;
+			blknum = fdt_offset / desc->blksz;
+			extra = fdt_offset % desc->blksz;
+			fdt_full_size = fdt_size + extra;
+			num_blks = DIV_ROUND_UP(fdt_full_size, desc->blksz);
+			fdt_base = ALIGN(base + len, 4);
+			fdt_base_buf = map_sysmem(fdt_base, fdt_size);
+			ret = blk_read(blk, blknum, num_blks, fdt_base_buf);
+			log_debug("fdt read foffset %lx blknum %lx full_size %lx num_blks %lx to %lx / %p: ret=%d\n",
+				  fdt_offset - 0x8000, blknum, fdt_full_size, num_blks,
+				  fdt_base, fdt_base_buf, ret);
+			if (ret != num_blks)
+				return log_msg_ret("rdf", -EIO);
+			if (extra) {
+				log_debug("move %p %p %lx\n", fdt_base_buf,
+					  fdt_base_buf + extra, fdt_size);
+				memmove(fdt_base_buf, fdt_base_buf + extra,
+					fdt_size);
+			}
+#if CONFIG_IS_ENABLED(RELOC_LOADER)
+			image->fdt_buf = fdt_base_buf;
+
+			ulong xpl_size;
+			ulong xpl_pad;
+			ulong fdt_start;
+
+			if (xpl_phase() == PHASE_TPL) {
+				xpl_size = binman_sym(ulong, u_boot_vpl_nodtb, size);
+				xpl_pad = binman_sym(ulong, u_boot_vpl_bss_pad, size);
+			} else {
+				xpl_size = binman_sym(ulong, u_boot_spl_nodtb, size);
+				xpl_pad = binman_sym(ulong, u_boot_spl_bss_pad, size);
+			}
+			fdt_start = image->load_addr + xpl_size + xpl_pad;
+			log_debug("load_addr %lx xpl_size %lx copy-to %lx\n",
+				  image->load_addr, xpl_size + xpl_pad,
+				  fdt_start);
+			image->fdt_start = map_sysmem(fdt_start, fdt_size);
+#endif
+		}
+	}
+	if (load_addrp)
+		*load_addrp = load_addr;
+	if (lenp)
+		*lenp = len;
+	if (namep) {
+		*namep = strdup(fdt_get_name(buf, node, NULL));
+		if (!namep)
+			return log_msg_ret("nam", -ENOMEM);
+	}
+
+	return 0;
+}
diff --git a/boot/vbe_common.h b/boot/vbe_common.h
new file mode 100644
index 00000000000..84117815a19
--- /dev/null
+++ b/boot/vbe_common.h
@@ -0,0 +1,137 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Verified Boot for Embedded (VBE) common functions
+ *
+ * Copyright 2024 Google LLC
+ * Written by Simon Glass <[email protected]>
+ */
+
+#ifndef __VBE_COMMON_H
+#define __VBE_COMMON_H
+
+#include <linux/types.h>
+
+struct spl_image_info;
+struct udevice;
+
+/*
+ * Controls whether we use a full bootmeth driver with VBE in this phase, or
+ * just access the information directly.
+ *
+ * For now VBE-simple uses the full bootmeth, but VBE-abrec does not, to reduce
+ * code size
+ */
+#define USE_BOOTMETH	CONFIG_IS_ENABLED(BOOTMETH_VBE_SIMPLE)
+
+enum {
+	MAX_VERSION_LEN		= 256,
+
+	NVD_HDR_VER_SHIFT	= 0,
+	NVD_HDR_VER_MASK	= 0xf,
+	NVD_HDR_SIZE_SHIFT	= 4,
+	NVD_HDR_SIZE_MASK	= 0xf << NVD_HDR_SIZE_SHIFT,
+
+	/* Firmware key-version is in the top 16 bits of fw_ver */
+	FWVER_KEY_SHIFT		= 16,
+	FWVER_FW_MASK		= 0xffff,
+
+	NVD_HDR_VER_CUR		= 1,	/* current version */
+};
+
+/**
+ * struct vbe_nvdata - basic storage format for non-volatile data
+ *
+ * This is used for all VBE methods
+ *
+ * @crc8: crc8 for the entire record except @crc8 field itself
+ * @hdr: header size and version (NVD_HDR_...)
+ * @spare1: unused, must be 0
+ * @fw_vernum: version and key version (FWVER_...)
+ * @flags: Flags controlling operation (enum vbe_flags)
+ */
+struct vbe_nvdata {
+	u8 crc8;
+	u8 hdr;
+	u16 spare1;
+	u32 fw_vernum;
+	u32 flags;
+	u8 spare2[0x34];
+};
+
+/**
+ * vbe_get_blk() - Obtain the block device to use for VBE
+ *
+ * Decodes the string to produce a block device
+ *
+ * @storage: String indicating the device to use, e.g. "mmc1"
+ * @blkp: Returns associated block device, on success
+ * Return 0 if OK, -ENODEV if @storage does not end with a number, -E2BIG if
+ * the device name is more than 15 characters, -ENXIO if the block device could
+ * not be found
+ */
+int vbe_get_blk(const char *storage, struct udevice **blkp);
+
+/**
+ * vbe_read_version() - Read version-string from a block device
+ *
+ * Reads the VBE version-string from a device. This function reads a single
+ * block from the device, so the string cannot be larger than that. It uses a
+ * temporary buffer for the read, then copies in up to @size bytes
+ *
+ * @blk: Device to read from
+ * @offset: Offset to read, in bytes
+ * @version: Place to put the string
+ * @max_size: Maximum size of @version
+ * Return: 0 if OK, -E2BIG if @max_size > block size, -EBADF if the offset is
+ * not block-aligned, -EIO if an I/O error occurred
+ */
+int vbe_read_version(struct udevice *blk, ulong offset, char *version,
+		     int max_size);
+
+/**
+ * vbe_read_nvdata() - Read non-volatile data from a block device
+ *
+ * Reads the VBE nvdata from a device. This function reads a single block from
+ * the device, so the nvdata cannot be larger than that.
+ *
+ * @blk: Device to read from
+ * @offset: Offset to read, in bytes
+ * @size: Number of bytes to read
+ * @buf: Buffer to hold the data
+ * Return: 0 if OK, -E2BIG if @size > block size, -EBADF if the offset is not
+ * block-aligned, -EIO if an I/O error occurred, -EPERM if the header version is
+ * incorrect, the header size is invalid or the data fails its CRC check
+ */
+int vbe_read_nvdata(struct udevice *blk, ulong offset, ulong size, u8 *buf);
+
+/**
+ * vbe_read_fit() - Read an image from a FIT
+ *
+ * This handles most of the VBE logic for reading from a FIT. It reads the FIT
+ * metadata, decides which image to load and loads it to a suitable address,
+ * ready for jumping to the next phase of VBE.
+ *
+ * This supports transition from VPL to SPL as well as SPL to U-Boot proper. For
+ * now, TPL->VPL is not supported.
+ *
+ * Both embedded and external data are supported for the FIT
+ *
+ * @blk: Block device containing FIT
+ * @area_offset: Byte offset of the VBE area in @blk containing the FIT
+ * @area_size: Size of the VBE area
+ * @image: SPL image to fill in with details of the loaded image, or NULL
+ * @load_addrp: If non-null, returns the address where the image was loaded
+ * @lenp: If non-null, returns the size of the image loaded, in bytes
+ * @namep: If non-null, returns the name of the FIT-image node that was loaded
+ *	(allocated by this function)
+ * Return: 0 if OK, -EINVAL if the area does not contain an FDT (the underlying
+ * format for FIT), -E2BIG if the FIT extends past @area_size, -ENOMEM if there
+ * was not space to allocate the image-node name, other error if a read error
+ * occurred (see blk_read()), or something went wrong with the actually
+ * FIT-parsing (see fit_image_load()).
+ */
+int vbe_read_fit(struct udevice *blk, ulong area_offset, ulong area_size,
+		 struct spl_image_info *image, ulong *load_addrp, ulong *lenp,
+		 char **namep);
+
+#endif /* __VBE_ABREC_H */
diff --git a/boot/vbe_simple.c b/boot/vbe_simple.c
index ed7b9598e38..c6766c532f2 100644
--- a/boot/vbe_simple.c
+++ b/boot/vbe_simple.c
@@ -18,70 +18,21 @@
 #include <vbe.h>
 #include <dm/device-internal.h>
 #include <dm/ofnode.h>
-#include <u-boot/crc.h>
 #include "vbe_simple.h"
 
-/** struct simple_nvdata - storage format for non-volatile data */
-struct simple_nvdata {
-	u8 crc8;
-	u8 hdr;
-	u16 spare1;
-	u32 fw_vernum;
-	u8 spare2[0x38];
-};
-
-static int simple_read_version(struct udevice *dev, struct blk_desc *desc,
-			       u8 *buf, struct simple_state *state)
+static int simple_read_nvdata(const struct simple_priv *priv,
+			      struct udevice *blk, struct simple_state *state)
 {
-	struct simple_priv *priv = dev_get_priv(dev);
-	int start;
-
-	if (priv->version_size > MMC_MAX_BLOCK_LEN)
-		return log_msg_ret("ver", -E2BIG);
-
-	start = priv->area_start + priv->version_offset;
-	if (start & (MMC_MAX_BLOCK_LEN - 1))
-		return log_msg_ret("get", -EBADF);
-	start /= MMC_MAX_BLOCK_LEN;
-
-	if (blk_dread(desc, start, 1, buf) != 1)
-		return log_msg_ret("read", -EIO);
-	strlcpy(state->fw_version, buf, MAX_VERSION_LEN);
-	log_debug("version=%s\n", state->fw_version);
+	ALLOC_CACHE_ALIGN_BUFFER(u8, buf, MMC_MAX_BLOCK_LEN);
+	const struct vbe_nvdata *nvd;
+	int ret;
 
-	return 0;
-}
+	ret = vbe_read_nvdata(blk, priv->area_start + priv->state_offset,
+			      priv->state_size, buf);
+	if (ret)
+		return log_msg_ret("nv", ret);
 
-static int simple_read_nvdata(struct udevice *dev, struct blk_desc *desc,
-			      u8 *buf, struct simple_state *state)
-{
-	struct simple_priv *priv = dev_get_priv(dev);
-	uint hdr_ver, hdr_size, size, crc;
-	const struct simple_nvdata *nvd;
-	int start;
-
-	if (priv->state_size > MMC_MAX_BLOCK_LEN)
-		return log_msg_ret("state", -E2BIG);
-
-	start = priv->area_start + priv->state_offset;
-	if (start & (MMC_MAX_BLOCK_LEN - 1))
-		return log_msg_ret("get", -EBADF);
-	start /= MMC_MAX_BLOCK_LEN;
-
-	if (blk_dread(desc, start, 1, buf) != 1)
-		return log_msg_ret("read", -EIO);
-	nvd = (struct simple_nvdata *)buf;
-	hdr_ver = (nvd->hdr & NVD_HDR_VER_MASK) >> NVD_HDR_VER_SHIFT;
-	hdr_size = (nvd->hdr & NVD_HDR_SIZE_MASK) >> NVD_HDR_SIZE_SHIFT;
-	if (hdr_ver != NVD_HDR_VER_CUR)
-		return log_msg_ret("hdr", -EPERM);
-	size = 1 << hdr_size;
-	if (size > sizeof(*nvd))
-		return log_msg_ret("sz", -ENOEXEC);
-
-	crc = crc8(0, buf + 1, size - 1);
-	if (crc != nvd->crc8)
-		return log_msg_ret("crc", -EPERM);
+	nvd = (struct vbe_nvdata *)buf;
 	state->fw_vernum = nvd->fw_vernum;
 
 	log_debug("version=%s\n", state->fw_version);
@@ -91,33 +42,20 @@ static int simple_read_nvdata(struct udevice *dev, struct blk_desc *desc,
 
 int vbe_simple_read_state(struct udevice *dev, struct simple_state *state)
 {
-	ALLOC_CACHE_ALIGN_BUFFER(u8, buf, MMC_MAX_BLOCK_LEN);
 	struct simple_priv *priv = dev_get_priv(dev);
-	struct blk_desc *desc;
-	char devname[16];
-	const char *end;
-	int devnum;
+	struct udevice *blk;
 	int ret;
 
-	/* First figure out the block device */
-	log_debug("storage=%s\n", priv->storage);
-	devnum = trailing_strtoln_end(priv->storage, NULL, &end);
-	if (devnum == -1)
-		return log_msg_ret("num", -ENODEV);
-	if (end - priv->storage >= sizeof(devname))
-		return log_msg_ret("end", -E2BIG);
-	strlcpy(devname, priv->storage, end - priv->storage + 1);
-	log_debug("dev=%s, %x\n", devname, devnum);
-
-	desc = blk_get_dev(devname, devnum);
-	if (!desc)
-		return log_msg_ret("get", -ENXIO);
-
-	ret = simple_read_version(dev, desc, buf, state);
+	ret = vbe_get_blk(priv->storage, &blk);
+	if (ret)
+		return log_msg_ret("blk", ret);
+
+	ret = vbe_read_version(blk, priv->area_start + priv->version_offset,
+			       state->fw_version, MAX_VERSION_LEN);
 	if (ret)
 		return log_msg_ret("ver", ret);
 
-	ret = simple_read_nvdata(dev, desc, buf, state);
+	ret = simple_read_nvdata(priv, blk, state);
 	if (ret)
 		return log_msg_ret("nvd", ret);
 
diff --git a/boot/vbe_simple.h b/boot/vbe_simple.h
index 56d319206f2..dc3f70052b0 100644
--- a/boot/vbe_simple.h
+++ b/boot/vbe_simple.h
@@ -9,20 +9,8 @@
 #ifndef __VBE_SIMPLE_H
 #define __VBE_SIMPLE_H
 
-enum {
-	MAX_VERSION_LEN		= 256,
-
-	NVD_HDR_VER_SHIFT	= 0,
-	NVD_HDR_VER_MASK	= 0xf,
-	NVD_HDR_SIZE_SHIFT	= 4,
-	NVD_HDR_SIZE_MASK	= 0xf << NVD_HDR_SIZE_SHIFT,
-
-	/* Firmware key-version is in the top 16 bits of fw_ver */
-	FWVER_KEY_SHIFT		= 16,
-	FWVER_FW_MASK		= 0xffff,
-
-	NVD_HDR_VER_CUR		= 1,	/* current version */
-};
+#include <linux/types.h>
+#include "vbe_common.h"
 
 /** struct simple_priv - information read from the device tree */
 struct simple_priv {
diff --git a/boot/vbe_simple_fw.c b/boot/vbe_simple_fw.c
index da9701f9eb9..cb5534fc731 100644
--- a/boot/vbe_simple_fw.c
+++ b/boot/vbe_simple_fw.c
@@ -8,6 +8,7 @@
 
 #define LOG_CATEGORY LOGC_BOOT
 
+#include <binman_sym.h>
 #include <bloblist.h>
 #include <bootdev.h>
 #include <bootflow.h>
@@ -17,13 +18,24 @@
 #include <image.h>
 #include <log.h>
 #include <mapmem.h>
-#include <memalign.h>
 #include <mmc.h>
 #include <spl.h>
 #include <vbe.h>
 #include <dm/device-internal.h>
+#include "vbe_common.h"
 #include "vbe_simple.h"
 
+#ifdef CONFIG_BOOTMETH_VBE_SIMPLE
+binman_sym_extern(ulong, vbe_a, image_pos);
+binman_sym_extern(ulong, vbe_a, size);
+#else
+binman_sym_declare(ulong, vbe_a, image_pos);
+binman_sym_declare(ulong, vbe_a, size);
+#endif
+
+binman_sym_declare(ulong, vpl, image_pos);
+binman_sym_declare(ulong, vpl, size);
+
 /**
  * vbe_simple_read_bootflow_fw() - Create a bootflow for firmware
  *
@@ -38,109 +50,26 @@
  */
 int vbe_simple_read_bootflow_fw(struct udevice *dev, struct bootflow *bflow)
 {
-	ALLOC_CACHE_ALIGN_BUFFER(u8, sbuf, MMC_MAX_BLOCK_LEN);
 	struct udevice *media = dev_get_parent(bflow->dev);
 	struct udevice *meth = bflow->method;
 	struct simple_priv *priv = dev_get_priv(meth);
-	const char *fit_uname, *fit_uname_config;
-	struct bootm_headers images = {};
-	ulong offset, size, blknum, addr, len, load_addr, num_blks;
-	enum image_phase_t phase;
-	struct blk_desc *desc;
+	ulong len, load_addr;
 	struct udevice *blk;
-	int node, ret;
-	void *buf;
+	int ret;
 
 	log_debug("media=%s\n", media->name);
 	ret = blk_get_from_parent(media, &blk);
 	if (ret)
 		return log_msg_ret("med", ret);
 	log_debug("blk=%s\n", blk->name);
-	desc = dev_get_uclass_plat(blk);
-
-	offset = priv->area_start + priv->skip_offset;
-
-	/* read in one block to find the FIT size */
-	blknum =  offset / desc->blksz;
-	log_debug("read at %lx, blknum %lx\n", offset, blknum);
-	ret = blk_read(blk, blknum, 1, sbuf);
-	if (ret < 0)
-		return log_msg_ret("rd", ret);
-
-	ret = fdt_check_header(sbuf);
-	if (ret < 0)
-		return log_msg_ret("fdt", -EINVAL);
-	size = fdt_totalsize(sbuf);
-	if (size > priv->area_size)
-		return log_msg_ret("fdt", -E2BIG);
-	log_debug("FIT size %lx\n", size);
-
-	/*
-	 * Load the FIT into the SPL memory. This is typically a FIT with
-	 * external data, so this is quite small, perhaps a few KB.
-	 */
-	addr = CONFIG_VAL(TEXT_BASE);
-	buf = map_sysmem(addr, size);
-	num_blks = DIV_ROUND_UP(size, desc->blksz);
-	log_debug("read %lx, %lx blocks to %lx / %p\n", size, num_blks, addr,
-		  buf);
-	ret = blk_read(blk, blknum, num_blks, buf);
-	if (ret < 0)
-		return log_msg_ret("rd", ret);
-
-	/* figure out the phase to load */
-	phase = IS_ENABLED(CONFIG_VPL_BUILD) ? IH_PHASE_SPL : IH_PHASE_U_BOOT;
-
-	/*
-	 * Load the image from the FIT. We ignore any load-address information
-	 * so in practice this simply locates the image in the external-data
-	 * region and returns its address and size. Since we only loaded the FIT
-	 * itself, only a part of the image will be present, at best.
-	 */
-	fit_uname = NULL;
-	fit_uname_config = NULL;
-	log_debug("loading FIT\n");
-	ret = fit_image_load(&images, addr, &fit_uname, &fit_uname_config,
-			     IH_ARCH_SANDBOX, image_ph(phase, IH_TYPE_FIRMWARE),
-			     BOOTSTAGE_ID_FIT_SPL_START, FIT_LOAD_IGNORED,
-			     &load_addr, &len);
-	if (ret < 0)
-		return log_msg_ret("ld", ret);
-	node = ret;
-	log_debug("loaded to %lx\n", load_addr);
-
-	/* For FIT external data, read in the external data */
-	if (load_addr + len > addr + size) {
-		ulong base, full_size;
-		void *base_buf;
-
-		/* Find the start address to load from */
-		base = ALIGN_DOWN(load_addr, desc->blksz);
-
-		/*
-		 * Get the total number of bytes to load, taking care of
-		 * block alignment
-		 */
-		full_size = load_addr + len - base;
-
-		/*
-		 * Get the start block number, number of blocks and the address
-		 * to load to, then load the blocks
-		 */
-		blknum = (offset + base - addr) / desc->blksz;
-		num_blks = DIV_ROUND_UP(full_size, desc->blksz);
-		base_buf = map_sysmem(base, full_size);
-		ret = blk_read(blk, blknum, num_blks, base_buf);
-		log_debug("read %lx %lx, %lx blocks to %lx / %p: ret=%d\n",
-			  blknum, full_size, num_blks, base, base_buf, ret);
-		if (ret < 0)
-			return log_msg_ret("rd", ret);
-	}
+
+	ret = vbe_read_fit(blk, priv->area_start + priv->skip_offset,
+			   priv->area_size, NULL, &load_addr, &len,
+			   &bflow->name);
+	if (ret)
+		return log_msg_ret("vbe", ret);
 
 	/* set up the bootflow with the info we obtained */
-	bflow->name = strdup(fdt_get_name(buf, node, NULL));
-	if (!bflow->name)
-		return log_msg_ret("name", -ENOMEM);
 	bflow->blk = blk;
 	bflow->buf = map_sysmem(load_addr, len);
 	bflow->size = len;
@@ -148,16 +77,14 @@ int vbe_simple_read_bootflow_fw(struct udevice *dev, struct bootflow *bflow)
 	return 0;
 }
 
-static int simple_load_from_image(struct spl_image_info *spl_image,
+static int simple_load_from_image(struct spl_image_info *image,
 				  struct spl_boot_device *bootdev)
 {
-	struct udevice *meth, *bdev;
-	struct simple_priv *priv;
-	struct bootflow bflow;
 	struct vbe_handoff *handoff;
 	int ret;
 
-	if (xpl_phase() != PHASE_VPL && xpl_phase() != PHASE_SPL)
+	if (xpl_phase() != PHASE_VPL && xpl_phase() != PHASE_SPL &&
+	    xpl_phase() != PHASE_TPL)
 		return -ENOENT;
 
 	ret = bloblist_ensure_size(BLOBLISTT_VBE, sizeof(struct vbe_handoff),
@@ -165,36 +92,64 @@ static int simple_load_from_image(struct spl_image_info *spl_image,
 	if (ret)
 		return log_msg_ret("ro", ret);
 
-	vbe_find_first_device(&meth);
-	if (!meth)
-		return log_msg_ret("vd", -ENODEV);
-	log_debug("vbe dev %s\n", meth->name);
-	ret = device_probe(meth);
-	if (ret)
-		return log_msg_ret("probe", ret);
-
-	priv = dev_get_priv(meth);
-	log_debug("simple %s\n", priv->storage);
-	ret = bootdev_find_by_label(priv->storage, &bdev, NULL);
-	if (ret)
-		return log_msg_ret("bd", ret);
-	log_debug("bootdev %s\n", bdev->name);
-
-	bootflow_init(&bflow, bdev, meth);
-	ret = bootmeth_read_bootflow(meth, &bflow);
-	log_debug("\nfw ret=%d\n", ret);
-	if (ret)
-		return log_msg_ret("rd", ret);
-
-	/* jump to the image */
-	spl_image->flags = SPL_SANDBOXF_ARG_IS_BUF;
-	spl_image->arg = bflow.buf;
-	spl_image->size = bflow.size;
-	log_debug("Image: %s at %p size %x\n", bflow.name, bflow.buf,
-		  bflow.size);
+	if (USE_BOOTMETH) {
+		struct udevice *meth, *bdev;
+		struct simple_priv *priv;
+		struct bootflow bflow;
+
+		vbe_find_first_device(&meth);
+		if (!meth)
+			return log_msg_ret("vd", -ENODEV);
+		log_debug("vbe dev %s\n", meth->name);
+		ret = device_probe(meth);
+		if (ret)
+			return log_msg_ret("probe", ret);
+
+		priv = dev_get_priv(meth);
+		log_debug("simple %s\n", priv->storage);
+		ret = bootdev_find_by_label(priv->storage, &bdev, NULL);
+		if (ret)
+			return log_msg_ret("bd", ret);
+		log_debug("bootdev %s\n", bdev->name);
+
+		bootflow_init(&bflow, bdev, meth);
+		ret = bootmeth_read_bootflow(meth, &bflow);
+		log_debug("\nfw ret=%d\n", ret);
+		if (ret)
+			return log_msg_ret("rd", ret);
 
-	/* this is not used from now on, so free it */
-	bootflow_free(&bflow);
+		/* jump to the image */
+		image->flags = SPL_SANDBOXF_ARG_IS_BUF;
+		image->arg = bflow.buf;
+		image->size = bflow.size;
+		log_debug("Image: %s at %p size %x\n", bflow.name, bflow.buf,
+			  bflow.size);
+
+		/* this is not used from now on, so free it */
+		bootflow_free(&bflow);
+	} else {
+		struct udevice *media, *blk;
+		ulong offset, size;
+
+		ret = uclass_get_device_by_seq(UCLASS_MMC, 1, &media);
+		if (ret)
+			return log_msg_ret("vdv", ret);
+		ret = blk_get_from_parent(media, &blk);
+		if (ret)
+			return log_msg_ret("med", ret);
+		if (xpl_phase() == PHASE_TPL) {
+			offset = binman_sym(ulong, vpl, image_pos);
+			size = binman_sym(ulong, vpl, size);
+		} else {
+			offset = binman_sym(ulong, vbe_a, image_pos);
+			size = binman_sym(ulong, vbe_a, size);
+			printf("offset=%lx\n", offset);
+		}
+
+		ret = vbe_read_fit(blk, offset, size, image, NULL, NULL, NULL);
+		if (ret)
+			return log_msg_ret("vbe", ret);
+	}
 
 	/* Record that VBE was used in this phase */
 	handoff->phases |= 1 << xpl_phase();