commonlib/region: Turn addrspace_32bit into a more official API

We had the addrspace_32bit rdev in prog_loaders.c for a while to help
represent memory ranges as an rdev, and we've found it useful for a
couple of things that have nothing to do with program loading. This
patch moves the concept straight into commonlib/region.c so it is no
longer anchored in such a weird place, and easier to use in unit tests.
Also expand the concept to the whole address space (there's no real need
to restrict it to 32 bits in 64-bit environments) and introduce an
rdev_chain_mem() helper function to make it a bit easier to use. Replace
some direct uses of struct mem_region_device with this new API where it
seems to make sense.

Signed-off-by: Julius Werner <jwerner@chromium.org>
Change-Id: Ie4c763b77f77d227768556a9528681d771a08dca
Reviewed-on: https://review.coreboot.org/c/coreboot/+/52533
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Aaron Durbin <adurbin@chromium.org>

src/commonlib/include/commonlib/region.h

@@ -164,14 +164,18 @@ static inline int rdev_chain_full(struct region_device *child,
 ssize_t rdev_relative_offset(const struct region_device *p,
 				const struct region_device *c);
 
+/* Helper functions to create an rdev that represents memory. */
+int rdev_chain_mem(struct region_device *child, const void *base, size_t size);
+int rdev_chain_mem_rw(struct region_device *child, void *base, size_t size);
+
 struct mem_region_device {
 	char *base;
 	struct region_device rdev;
 };
 
-/* Initialize at runtime a mem_region_device. This would be used when
- * the base and size are dynamic or can't be known during linking.
- * There are two variants: read-only and read-write. */
+/* Initialize at runtime a mem_region_device. Should only be used for mappings
+   that need to fit right up to the edge of the physical address space. Most use
+   cases will want to use rdev_chain_mem() instead. */
 void mem_region_device_ro_init(struct mem_region_device *mdev, void *base,
 				size_t size);
 
@@ -182,7 +186,8 @@ extern const struct region_device_ops mem_rdev_ro_ops;
 
 extern const struct region_device_ops mem_rdev_rw_ops;
 
-/* Statically initialize mem_region_device. */
+/* Statically initialize mem_region_device. Should normally only be used for
+   const globals. Most use cases will want to use rdev_chain_mem() instead. */
 #define MEM_REGION_DEV_INIT(base_, size_, ops_)				\
 	{								\
 		.base = (void *)(base_),				\

src/commonlib/region.c

@@ -287,6 +287,19 @@ const struct region_device_ops mem_rdev_rw_ops = {
 	.eraseat = mdev_eraseat,
 };
 
+static const struct mem_region_device mem_rdev = MEM_REGION_DEV_RO_INIT(0, ~(size_t)0);
+static const struct mem_region_device mem_rdev_rw = MEM_REGION_DEV_RW_INIT(0, ~(size_t)0);
+
+int rdev_chain_mem(struct region_device *child, const void *base, size_t size)
+{
+	return rdev_chain(child, &mem_rdev.rdev, (uintptr_t)base, size);
+}
+
+int rdev_chain_mem_rw(struct region_device *child, void *base, size_t size)
+{
+	return rdev_chain(child, &mem_rdev_rw.rdev, (uintptr_t)base, size);
+}
+
 void *mmap_helper_rdev_mmap(const struct region_device *rd, size_t offset,
 				size_t size)
 {

src/drivers/elog/elog.c

@@ -44,7 +44,7 @@ struct elog_state {
 
 	struct region_device nv_dev;
 	/* Device that mirrors the eventlog in memory. */
-	struct mem_region_device mirror_dev;
+	struct region_device mirror_dev;
 
 	enum elog_init_state elog_initialized;
 };
@@ -56,7 +56,7 @@ static uint8_t elog_mirror_buf[ELOG_SIZE];
 
 static inline struct region_device *mirror_dev_get(void)
 {
-	return &elog_state.mirror_dev.rdev;
+	return &elog_state.mirror_dev;
 }
 
 static size_t elog_events_start(void)
@@ -798,8 +798,7 @@ int elog_init(void)
 		printk(BIOS_ERR, "ELOG: Unable to allocate backing store\n");
 		return -1;
 	}
-	mem_region_device_rw_init(&elog_state.mirror_dev, mirror_buffer,
-				  elog_size);
+	rdev_chain_mem_rw(&elog_state.mirror_dev, mirror_buffer, elog_size);
 
 	/*
 	 * Mark as initialized to allow elog_init() to be called and deemed

src/drivers/intel/gma/opregion.c

@@ -128,16 +128,14 @@ static enum cb_err locate_vbt_vbios(const u8 *vbios, struct region_device *rdev)
 	size_t offset;
 
 	// FIXME: caller should supply a region_device instead of vbios pointer
-	if (rdev_chain(&rd, &addrspace_32bit.rdev, (uintptr_t)vbios,
-	    sizeof(*oprom)))
+	if (rdev_chain_mem(&rd, vbios, sizeof(*oprom)))
 		return CB_ERR;
 
 	if (rdev_readat(&rd, &opromsize, offsetof(optionrom_header_t, size),
 	    sizeof(opromsize)) != sizeof(opromsize) || !opromsize)
 		return CB_ERR;
 
-	if (rdev_chain(&rd, &addrspace_32bit.rdev, (uintptr_t)vbios,
-	    opromsize * 512))
+	if (rdev_chain_mem(&rd, vbios, opromsize * 512))
 		return CB_ERR;
 
 	oprom = rdev_mmap(&rd, 0, sizeof(*oprom));
@@ -200,8 +198,7 @@ static enum cb_err locate_vbt_cbfs(struct region_device *rdev)
 	if (vbt == NULL)
 		return CB_ERR;
 
-	if (rdev_chain(rdev, &addrspace_32bit.rdev, (uintptr_t)vbt,
-	    vbt_data_size))
+	if (rdev_chain_mem(rdev, vbt, vbt_data_size))
 		return CB_ERR;
 
 	printk(BIOS_INFO, "GMA: Found VBT in CBFS\n");

src/drivers/smmstore/store.c

@@ -267,14 +267,14 @@ int smmstore_clear_region(void)
 /* Implementation of Version 2 */
 
 static bool store_initialized;
-static struct mem_region_device mdev_com_buf;
+static struct region_device mdev_com_buf;
 
 static int smmstore_rdev_chain(struct region_device *rdev)
 {
 	if (!store_initialized)
 		return -1;
 
-	return rdev_chain_full(rdev, &mdev_com_buf.rdev);
+	return rdev_chain_full(rdev, &mdev_com_buf);
 }
 
 /**
@@ -289,7 +289,7 @@ int smmstore_init(void *buf, size_t len)
 	if (store_initialized)
 		return -1;
 
-	mem_region_device_rw_init(&mdev_com_buf, buf, len);
+	rdev_chain_mem_rw(&mdev_com_buf, buf, len);
 
 	store_initialized = true;
 

src/drivers/vpd/vpd.c

@@ -101,10 +101,8 @@ static int init_vpd_rdevs_from_cbmem(void)
 	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);
+	rdev_chain_mem(&ro_vpd, cbmem->blob, cbmem->ro_size);
+	rdev_chain_mem(&rw_vpd, cbmem->blob + cbmem->ro_size, cbmem->rw_size);
 
 	return 0;
 }

src/include/program_loading.h

@@ -91,15 +91,11 @@ static inline void *prog_entry_arg(const struct prog *prog)
 	return prog->arg;
 }
 
-/* region_device representing the 32-bit flat address space. */
-extern const struct mem_region_device addrspace_32bit;
-
 /* Can be used to get an rdev representation of program area in memory. */
 static inline void prog_chain_rdev(const struct prog *prog,
 				   struct region_device *rdev_out)
 {
-	rdev_chain(rdev_out, &addrspace_32bit.rdev,
-		   (uintptr_t)prog->start, prog->size);
+	rdev_chain_mem(rdev_out, prog->start, prog->size);
 }
 
 static inline void prog_set_area(struct prog *prog, void *start, size_t size)

src/lib/cbfs.c

@@ -89,7 +89,7 @@ int cbfs_boot_locate(struct cbfsf *fh, const char *name, uint32_t *type)
 		return -1;
 
 	size_t msize = be32toh(fh->mdata.h.offset);
-	if (rdev_chain(&fh->metadata, &addrspace_32bit.rdev, (uintptr_t)&fh->mdata, msize))
+	if (rdev_chain_mem(&fh->metadata, &fh->mdata, msize))
 		return -1;
 
 	if (type) {
@@ -436,7 +436,7 @@ cb_err_t cbfs_prog_stage_load(struct prog *pstage)
 		void *compr_start = prog_start(pstage) + prog_size(pstage) - in_size;
 		if (rdev_readat(&rdev, compr_start, 0, in_size) != in_size)
 			return CB_ERR;
-		rdev_chain(&rdev, &addrspace_32bit.rdev, (uintptr_t)compr_start, in_size);
+		rdev_chain_mem(&rdev, compr_start, in_size);
 	}
 
 	size_t fsize = cbfs_load_and_decompress(&rdev, prog_start(pstage), prog_size(pstage),

src/lib/fmap.c

@@ -16,7 +16,7 @@
  */
 
 static int fmap_print_once;
-static struct mem_region_device fmap_cache;
+static struct region_device fmap_cache;
 
 #define print_once(...) do { \
 		if (!fmap_print_once) \
@@ -53,7 +53,7 @@ static void report(const struct fmap *fmap)
 	fmap_print_once = 1;
 }
 
-static void setup_preram_cache(struct mem_region_device *cache_mrdev)
+static void setup_preram_cache(struct region_device *cache_rdev)
 {
 	if (CONFIG(NO_FMAP_CACHE))
 		return;
@@ -99,7 +99,7 @@ static void setup_preram_cache(struct mem_region_device *cache_mrdev)
 	report(fmap);
 
 register_cache:
-	mem_region_device_ro_init(cache_mrdev, fmap, FMAP_SIZE);
+	rdev_chain_mem(cache_rdev, fmap, FMAP_SIZE);
 }
 
 static int find_fmap_directory(struct region_device *fmrd)
@@ -109,10 +109,10 @@ static int find_fmap_directory(struct region_device *fmrd)
 	size_t offset = FMAP_OFFSET;
 
 	/* Try FMAP cache first */
-	if (!region_device_sz(&fmap_cache.rdev))
+	if (!region_device_sz(&fmap_cache))
 		setup_preram_cache(&fmap_cache);
-	if (region_device_sz(&fmap_cache.rdev))
-		return rdev_chain_full(fmrd, &fmap_cache.rdev);
+	if (region_device_sz(&fmap_cache))
+		return rdev_chain_full(fmrd, &fmap_cache);
 
 	boot_device_init();
 	boot = boot_device_ro();
@@ -281,7 +281,7 @@ static void fmap_register_cbmem_cache(int unused)
 	if (!e)
 		return;
 
-	mem_region_device_ro_init(&fmap_cache, cbmem_entry_start(e), cbmem_entry_size(e));
+	rdev_chain_mem(&fmap_cache, cbmem_entry_start(e), cbmem_entry_size(e));
 }
 
 /*

src/lib/prog_loaders.c

@@ -16,10 +16,6 @@
 #include <timestamp.h>
 #include <security/vboot/vboot_common.h>
 
-/* Only can represent up to 1 byte less than size_t. */
-const struct mem_region_device addrspace_32bit =
-	MEM_REGION_DEV_RO_INIT(0, ~0UL);
-
 void run_romstage(void)
 {
 	struct prog romstage =

src/soc/intel/apollolake/mmap_boot.c

@@ -42,7 +42,7 @@
 
 static size_t bios_size;
 
-static struct mem_region_device shadow_dev;
+static struct region_device shadow_dev;
 static struct xlate_region_device real_dev;
 static struct xlate_window real_dev_window;
 
@@ -67,10 +67,9 @@ static void bios_mmap_init(void)
 	 */
 	bios_mapped_size = size - 256 * KiB;
 
-	mem_region_device_ro_init(&shadow_dev, (void *)base,
-			       bios_mapped_size);
+	rdev_chain_mem(&shadow_dev, (void *)base, bios_mapped_size);
 
-	xlate_window_init(&real_dev_window, &shadow_dev.rdev, start, bios_mapped_size);
+	xlate_window_init(&real_dev_window, &shadow_dev, start, bios_mapped_size);
 	xlate_region_device_ro_init(&real_dev, 1, &real_dev_window, CONFIG_ROM_SIZE);
 
 	bios_size = size;
@@ -98,7 +97,7 @@ uint32_t spi_flash_get_mmap_windows(struct flash_mmap_window *table)
 	bios_mmap_init();
 
 	table->flash_base = region_offset(&real_dev_window.sub_region);
-	table->host_base = (uintptr_t)rdev_mmap_full(&shadow_dev.rdev);
+	table->host_base = (uintptr_t)rdev_mmap_full(&shadow_dev);
 	table->size = region_sz(&real_dev_window.sub_region);
 
 	return 1;

src/soc/samsung/exynos5250/alternate_cbfs.c

@@ -92,8 +92,8 @@ static int sdmmc_cbfs_open(void)
 	return 0;
 }
 
-static struct mem_region_device alternate_rdev =
-	MEM_REGION_DEV_RO_INIT(NULL, 0);
+const static struct mem_region_device alternate_rdev =
+	MEM_REGION_DEV_RO_INIT(_cbfs_cache, REGION_SIZE(cbfs_cache));
 
 const struct region_device *boot_device_ro(void)
 {
@@ -114,9 +114,6 @@ const struct region_device *boot_device_ro(void)
 
 void boot_device_init(void)
 {
-	mem_region_device_ro_init(&alternate_rdev, _cbfs_cache,
-			REGION_SIZE(cbfs_cache));
-
 	if (*iram_secondary_base == SECONDARY_BASE_BOOT_USB) {
 		printk(BIOS_DEBUG, "Using Exynos alternate boot mode USB A-A\n");
 		usb_cbfs_open();

src/soc/samsung/exynos5420/alternate_cbfs.c

@@ -100,7 +100,7 @@ static int sdmmc_cbfs_open(void)
 }
 
 static struct mem_region_device alternate_rdev =
-	MEM_REGION_DEV_RO_INIT(NULL, 0);
+	MEM_REGION_DEV_RO_INIT(_cbfs_cache, REGION_SIZE(cbfs_cache));
 
 const struct region_device *boot_device_ro(void)
 {
@@ -121,9 +121,6 @@ const struct region_device *boot_device_ro(void)
 
 void boot_device_init(void)
 {
-	mem_region_device_ro_init(&alternate_rdev, _cbfs_cache,
-			REGION_SIZE(cbfs_cache));
-
 	if (*iram_secondary_base == SECONDARY_BASE_BOOT_USB) {
 		printk(BIOS_DEBUG, "Using Exynos alternate boot mode USB A-A\n");
 		usb_cbfs_open();

tests/commonlib/region-test.c

@@ -333,32 +333,33 @@ static void test_mem_rdev(void **state)
 	u8 backing[size];
 	u8 scratch[size];
 	int i;
-	struct mem_region_device mem = MEM_REGION_DEV_RW_INIT(backing, size);
+	struct region_device mem;
+	rdev_chain_mem_rw(&mem, backing, size);
 
 	/* Test writing to and reading from full mapping. */
 	memset(backing, 0xa5, size);
-	u8 *mapping = rdev_mmap_full(&mem.rdev);
+	u8 *mapping = rdev_mmap_full(&mem);
 	assert_non_null(mapping);
 	for (i = 0; i < size; i++)
 		assert_int_equal(mapping[i], 0xa5);
 	memset(mapping, 0x5a, size);
 	for (i = 0; i < size; i++)
 		assert_int_equal(backing[i], 0x5a);
-	assert_int_equal(rdev_munmap(&mem.rdev, mapping), 0);
+	assert_int_equal(rdev_munmap(&mem, mapping), 0);
 
 	/* Test read/write/erase of single bytes. */
 	for (i = 0; i < size; i++) {
 		u8 val = i + 0xaa;
 		scratch[0] = val;
-		assert_int_equal(rdev_writeat(&mem.rdev, &scratch, i, 1), 1);
+		assert_int_equal(rdev_writeat(&mem, &scratch, i, 1), 1);
 		assert_int_equal(backing[i], val);
 		assert_int_equal(scratch[0], val);
 		val = i + 0x55;
 		backing[i] = val;
-		assert_int_equal(rdev_readat(&mem.rdev, &scratch, i, 1), 1);
+		assert_int_equal(rdev_readat(&mem, &scratch, i, 1), 1);
 		assert_int_equal(scratch[0], val);
 		assert_int_equal(backing[i], val);
-		assert_int_equal(rdev_eraseat(&mem.rdev, i, 1), 1);
+		assert_int_equal(rdev_eraseat(&mem, i, 1), 1);
 		assert_int_equal(backing[i], 0);
 	}
 
@@ -368,25 +369,25 @@ static void test_mem_rdev(void **state)
 	memset(backing, 0, size);
 	memset(scratch, 0, size);
 	memset(scratch + offs, 0x39, chunk);
-	assert_int_equal(rdev_writeat(&mem.rdev, scratch + offs, offs, chunk), chunk);
+	assert_int_equal(rdev_writeat(&mem, scratch + offs, offs, chunk), chunk);
 	assert_memory_equal(backing, scratch, size);
 	memset(backing, 0, size);
-	assert_int_equal(rdev_readat(&mem.rdev, scratch + offs, offs, chunk), chunk);
+	assert_int_equal(rdev_readat(&mem, scratch + offs, offs, chunk), chunk);
 	assert_memory_equal(backing, scratch, size);
 	memset(scratch + offs + 1, 0, chunk - 1);
-	assert_int_equal(rdev_eraseat(&mem.rdev, offs + 1, chunk - 1), chunk - 1);
+	assert_int_equal(rdev_eraseat(&mem, offs + 1, chunk - 1), chunk - 1);
 	assert_memory_equal(backing, scratch, size);
 
 	/* Test mapping of larger chunk. */
 	memset(backing, 0, size);
-	mapping = rdev_mmap(&mem.rdev, offs, chunk);
+	mapping = rdev_mmap(&mem, offs, chunk);
 	assert_non_null(mapping);
 	memset(scratch, 0x93, size);
 	memcpy(mapping, scratch, chunk);
 	memset(scratch, 0, size);
 	memset(scratch + offs, 0x93, chunk);
 	assert_memory_equal(backing, scratch, size);
-	assert_int_equal(rdev_munmap(&mem.rdev, mapping), 0);
+	assert_int_equal(rdev_munmap(&mem, mapping), 0);
 	assert_memory_equal(backing, scratch, size);
 }