drivers: Move (un)map_flash_region to par/spi/opaque_master

Move (un)map_flash_region function pointers from programmer_entry to
par_master, spi_master, and opaque_master.  This enables programmers to
specify a different mapper per bus, which is needed for the internal
programmer.  Mapping is closely tied to the way the memory is accessed
using the other functions in the bus master structs.

Validate that FWH/LPC programmers provide specialized mapping in
register_par_master(); this is needed for chips with
FEATURE_REGISTERMAP, which only exist on FWH or LPC buses.

programmer.c: Update comment in fallback_map(), NULL return is the
desired behavior.

Test: Read firmware on SB600 Promontory mainboard (requires physmap)
Test: Read firmware externally with ft2232_spi
Test: Read firmware on ICH hwseq, verify physmap still occurs

Change-Id: I9c3df6ae260bcdb246dfb0cd8e043919609b014b
Signed-off-by: Jonathon Hall <jonathon.hall@puri.sm>
Co-Authored-by: Edward O'Callaghan <quasisec@google.com>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/67695
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>

13 files changed, 119 insertions, 41 deletions

diff --git a/atavia.c b/atavia.c
index 66bca65e..5cbe058f 100644
--- a/atavia.c
+++ b/atavia.c
@@ -133,6 +133,7 @@ static uint8_t atavia_chip_readb(const struct flashctx *flash, const chipaddr ad
 }
 
 static const struct par_master lpc_master_atavia = {
+	.map_flash_region	= atavia_map,
 	.chip_readb	= atavia_chip_readb,
 	.chip_readw	= fallback_chip_readw,
 	.chip_readl	= fallback_chip_readl,
@@ -189,5 +190,4 @@ const struct programmer_entry programmer_atavia = {
 	.type			= PCI,
 	.devs.dev		= ata_via,
 	.init			= atavia_init,
-	.map_flash_region	= atavia_map,
 };
diff --git a/dummyflasher.c b/dummyflasher.c
index b2a41c6e..ad734f08 100644
--- a/dummyflasher.c
+++ b/dummyflasher.c
@@ -920,6 +920,8 @@ static int dummy_shutdown(void *data)
 }
 
 static const struct spi_master spi_master_dummyflasher = {
+	.map_flash_region	= dummy_map,
+	.unmap_flash_region	= dummy_unmap,
 	.features	= SPI_MASTER_4BA,
 	.max_data_read	= MAX_DATA_READ_UNLIMITED,
 	.max_data_write	= MAX_DATA_UNSPECIFIED,
@@ -932,6 +934,8 @@ static const struct spi_master spi_master_dummyflasher = {
 };
 
 static const struct par_master par_master_dummyflasher = {
+	.map_flash_region	= dummy_map,
+	.unmap_flash_region	= dummy_unmap,
 	.chip_readb	= dummy_chip_readb,
 	.chip_readw	= dummy_chip_readw,
 	.chip_readl	= dummy_chip_readl,
@@ -943,6 +947,8 @@ static const struct par_master par_master_dummyflasher = {
 };
 
 static const struct opaque_master opaque_master_dummyflasher = {
+	.map_flash_region	= dummy_map,
+	.unmap_flash_region	= dummy_unmap,
 	.probe	= probe_variable_size,
 	.read	= dummy_opaque_read,
 	.write	= dummy_opaque_write,
@@ -1424,6 +1430,4 @@ const struct programmer_entry programmer_dummy = {
 				/* FIXME */
 	.devs.note		= "Dummy device, does nothing and logs all accesses\n",
 	.init			= dummy_init,
-	.map_flash_region	= dummy_map,
-	.unmap_flash_region	= dummy_unmap,
 };
diff --git a/flashrom.c b/flashrom.c
index f74b79a9..90899f05 100644
--- a/flashrom.c
+++ b/flashrom.c
@@ -203,27 +203,62 @@ int programmer_shutdown(void)
 	return ret;
 }
 
-void *programmer_map_flash_region(const char *descr, uintptr_t phys_addr, size_t len)
+void *master_map_flash_region(const struct registered_master *mst,
+			      const char *descr, uintptr_t phys_addr,
+			      size_t len)
 {
+	/* Check the bus master for a specialized map_flash_region; default to
+	 * fallback if it does not specialize it
+	 */
+	void *(*map_flash_region) (const char *descr, uintptr_t phys_addr, size_t len) = NULL;
+	if (mst->buses_supported & BUS_PROG)
+		map_flash_region = mst->opaque.map_flash_region;
+	else if (mst->buses_supported & BUS_SPI)
+		map_flash_region = mst->spi.map_flash_region;
+	else if (mst->buses_supported & BUS_NONSPI)
+		map_flash_region = mst->par.map_flash_region;
+
 	void *ret;
-	if (programmer->map_flash_region)
-		ret = programmer->map_flash_region(descr, phys_addr, len);
+	if (map_flash_region)
+		ret = map_flash_region(descr, phys_addr, len);
 	else
 		ret = fallback_map(descr, phys_addr, len);
 	msg_gspew("%s: mapping %s from 0x%0*" PRIxPTR " to 0x%0*" PRIxPTR "\n",
-		  __func__, descr, PRIxPTR_WIDTH, phys_addr, PRIxPTR_WIDTH, (uintptr_t) ret);
+		__func__, descr, PRIxPTR_WIDTH, phys_addr, PRIxPTR_WIDTH, (uintptr_t) ret);
 	return ret;
 }
 
-void programmer_unmap_flash_region(void *virt_addr, size_t len)
+void master_unmap_flash_region(const struct registered_master *mst,
+			       void *virt_addr, size_t len)
 {
-	if (programmer->unmap_flash_region)
-		programmer->unmap_flash_region(virt_addr, len);
+	void (*unmap_flash_region) (void *virt_addr, size_t len) = NULL;
+	if (mst->buses_supported & BUS_PROG)
+		unmap_flash_region = mst->opaque.unmap_flash_region;
+	else if (mst->buses_supported & BUS_SPI)
+		unmap_flash_region = mst->spi.unmap_flash_region;
+	else if (mst->buses_supported & BUS_NONSPI)
+		unmap_flash_region = mst->par.unmap_flash_region;
+
+	if (unmap_flash_region)
+		unmap_flash_region(virt_addr, len);
 	else
 		fallback_unmap(virt_addr, len);
 	msg_gspew("%s: unmapped 0x%0*" PRIxPTR "\n", __func__, PRIxPTR_WIDTH, (uintptr_t)virt_addr);
 }
 
+static bool master_uses_physmap(const struct registered_master *mst)
+{
+#if CONFIG_INTERNAL == 1
+	if (mst->buses_supported & BUS_PROG)
+		return mst->opaque.map_flash_region == physmap;
+	else if (mst->buses_supported & BUS_SPI)
+		return mst->spi.map_flash_region == physmap;
+	else if (mst->buses_supported & BUS_NONSPI)
+		return mst->par.map_flash_region == physmap;
+#endif
+	return false;
+}
+
 void programmer_delay(unsigned int usecs)
 {
 	if (usecs > 0) {
@@ -647,13 +682,13 @@ unsigned int count_max_decode_exceedings(const struct flashctx *flash)
 void unmap_flash(struct flashctx *flash)
 {
 	if (flash->virtual_registers != (chipaddr)ERROR_PTR) {
-		programmer_unmap_flash_region((void *)flash->virtual_registers, flash->chip->total_size * 1024);
+		master_unmap_flash_region(flash->mst, (void *)flash->virtual_registers, flash->chip->total_size * 1024);
 		flash->physical_registers = 0;
 		flash->virtual_registers = (chipaddr)ERROR_PTR;
 	}
 
 	if (flash->virtual_memory != (chipaddr)ERROR_PTR) {
-		programmer_unmap_flash_region((void *)flash->virtual_memory, flash->chip->total_size * 1024);
+		master_unmap_flash_region(flash->mst, (void *)flash->virtual_memory, flash->chip->total_size * 1024);
 		flash->physical_memory = 0;
 		flash->virtual_memory = (chipaddr)ERROR_PTR;
 	}
@@ -673,7 +708,7 @@ int map_flash(struct flashctx *flash)
 
 	const chipsize_t size = flash->chip->total_size * 1024;
 	uintptr_t base = flashbase ? flashbase : (0xffffffff - size + 1);
-	void *addr = programmer_map_flash_region(flash->chip->name, base, size);
+	void *addr = master_map_flash_region(flash->mst, flash->chip->name, base, size);
 	if (addr == ERROR_PTR) {
 		msg_perr("Could not map flash chip %s at 0x%0*" PRIxPTR ".\n",
 			 flash->chip->name, PRIxPTR_WIDTH, base);
@@ -687,7 +722,7 @@ int map_flash(struct flashctx *flash)
 	 * Ignore these problems for now and always report success. */
 	if (flash->chip->feature_bits & FEATURE_REGISTERMAP) {
 		base = 0xffffffff - size - 0x400000 + 1;
-		addr = programmer_map_flash_region("flash chip registers", base, size);
+		addr = master_map_flash_region(flash->mst, "flash chip registers", base, size);
 		if (addr == ERROR_PTR) {
 			msg_pdbg2("Could not map flash chip registers %s at 0x%0*" PRIxPTR ".\n",
 				 flash->chip->name, PRIxPTR_WIDTH, base);
@@ -831,12 +866,10 @@ notfound:
 	msg_cinfo("%s %s flash chip \"%s\" (%d kB, %s) ", force ? "Assuming" : "Found",
 		  flash->chip->vendor, flash->chip->name, flash->chip->total_size, tmp);
 	free(tmp);
-#if CONFIG_INTERNAL == 1
-	if (programmer->map_flash_region == physmap)
+	if (master_uses_physmap(mst))
 		msg_cinfo("mapped at physical address 0x%0*" PRIxPTR ".\n",
 			  PRIxPTR_WIDTH, flash->physical_memory);
 	else
-#endif
 		msg_cinfo("on %s.\n", programmer->name);
 
 	/* Flash registers may more likely not be mapped if the chip was forced.
diff --git a/ichspi.c b/ichspi.c
index fa15a953..7e7967e0 100644
--- a/ichspi.c
+++ b/ichspi.c
@@ -1773,6 +1773,8 @@ static const struct spi_master spi_master_ich7 = {
 	.max_data_write	= 64,
 	.command	= ich_spi_send_command,
 	.multicommand	= ich_spi_send_multicommand,
+	.map_flash_region	= physmap,
+	.unmap_flash_region	= physunmap,
 	.read		= default_spi_read,
 	.write_256	= default_spi_write_256,
 	.write_aai	= default_spi_write_aai,
@@ -1783,6 +1785,8 @@ static const struct spi_master spi_master_ich9 = {
 	.max_data_write	= 64,
 	.command	= ich_spi_send_command,
 	.multicommand	= ich_spi_send_multicommand,
+	.map_flash_region	= physmap,
+	.unmap_flash_region	= physunmap,
 	.read		= default_spi_read,
 	.write_256	= default_spi_write_256,
 	.write_aai	= default_spi_write_aai,
@@ -1790,6 +1794,8 @@ static const struct spi_master spi_master_ich9 = {
 };
 
 static const struct opaque_master opaque_master_ich_hwseq = {
+	.map_flash_region	= physmap,
+	.unmap_flash_region	= physunmap,
 	.max_data_read	= 64,
 	.max_data_write	= 64,
 	.probe		= ich_hwseq_probe,
@@ -2199,6 +2205,8 @@ static const struct spi_master spi_master_via = {
 	.max_data_write	= 16,
 	.command	= ich_spi_send_command,
 	.multicommand	= ich_spi_send_multicommand,
+	.map_flash_region	= physmap,
+	.unmap_flash_region	= physunmap,
 	.read		= default_spi_read,
 	.write_256	= default_spi_write_256,
 	.write_aai	= default_spi_write_aai,
diff --git a/include/flash.h b/include/flash.h
index 79aaa64a..c696f64e 100644
--- a/include/flash.h
+++ b/include/flash.h
@@ -55,8 +55,11 @@ typedef uintptr_t chipaddr;
 #define PRIxPTR_WIDTH ((int)(sizeof(uintptr_t)*2))
 
 int register_shutdown(int (*function) (void *data), void *data);
-void *programmer_map_flash_region(const char *descr, uintptr_t phys_addr, size_t len);
-void programmer_unmap_flash_region(void *virt_addr, size_t len);
+struct registered_master;
+void *master_map_flash_region(const struct registered_master *mast,
+			      const char *descr, uintptr_t phys_addr, size_t len);
+void master_unmap_flash_region(const struct registered_master *mast,
+			       void *virt_addr, size_t len);
 void programmer_delay(unsigned int usecs);
 
 #define ARRAY_SIZE(a) (sizeof(a) / sizeof((a)[0]))
diff --git a/include/programmer.h b/include/programmer.h
index 17abfd22..4b5bf92a 100644
--- a/include/programmer.h
+++ b/include/programmer.h
@@ -50,9 +50,6 @@ struct programmer_entry {
 
 	int (*init) (const struct programmer_cfg *cfg);
 
-	void *(*map_flash_region) (const char *descr, uintptr_t phys_addr, size_t len);
-	void (*unmap_flash_region) (void *virt_addr, size_t len);
-
 	void (*delay) (unsigned int usecs);
 };
 
@@ -309,6 +306,8 @@ struct spi_master {
 	int (*multicommand)(const struct flashctx *flash, struct spi_command *cmds);
 
 	/* Optimized functions for this master */
+	void *(*map_flash_region) (const char *descr, uintptr_t phys_addr, size_t len);
+	void (*unmap_flash_region) (void *virt_addr, size_t len);
 	int (*read)(struct flashctx *flash, uint8_t *buf, unsigned int start, unsigned int len);
 	int (*write_256)(struct flashctx *flash, const uint8_t *buf, unsigned int start, unsigned int len);
 	int (*write_aai)(struct flashctx *flash, const uint8_t *buf, unsigned int start, unsigned int len);
@@ -399,6 +398,8 @@ int wbsio_check_for_spi(void);
 
 /* opaque.c */
 struct opaque_master {
+	void *(*map_flash_region) (const char *descr, uintptr_t phys_addr, size_t len);
+	void (*unmap_flash_region) (void *virt_addr, size_t len);
 	int max_data_read;
 	int max_data_write;
 	/* Specific functions for this master */
@@ -424,6 +425,8 @@ int register_opaque_master(const struct opaque_master *mst, void *data);
 
 /* parallel.c */
 struct par_master {
+	void *(*map_flash_region) (const char *descr, uintptr_t phys_addr, size_t len);
+	void (*unmap_flash_region) (void *virt_addr, size_t len);
 	void (*chip_writeb) (const struct flashctx *flash, uint8_t val, chipaddr addr);
 	void (*chip_writew) (const struct flashctx *flash, uint16_t val, chipaddr addr);
 	void (*chip_writel) (const struct flashctx *flash, uint32_t val, chipaddr addr);
diff --git a/internal.c b/internal.c
index 2558cdef..8c834e2c 100644
--- a/internal.c
+++ b/internal.c
@@ -107,6 +107,8 @@ static void internal_chip_readn(const struct flashctx *flash, uint8_t *buf,
 }
 
 static const struct par_master par_master_internal = {
+	.map_flash_region	= physmap,
+	.unmap_flash_region	= physunmap,
 	.chip_readb	= internal_chip_readb,
 	.chip_readw	= internal_chip_readw,
 	.chip_readl	= internal_chip_readl,
@@ -347,6 +349,4 @@ const struct programmer_entry programmer_internal = {
 	.type			= OTHER,
 	.devs.note		= NULL,
 	.init			= internal_init,
-	.map_flash_region	= physmap,
-	.unmap_flash_region	= physunmap,
 };
diff --git a/it87spi.c b/it87spi.c
index 85da1507..5acaea69 100644
--- a/it87spi.c
+++ b/it87spi.c
@@ -315,6 +315,8 @@ static const struct spi_master spi_master_it87xx = {
 	.max_data_write	= MAX_DATA_UNSPECIFIED,
 	.command	= it8716f_spi_send_command,
 	.multicommand	= default_spi_send_multicommand,
+	.map_flash_region	= physmap,
+	.unmap_flash_region	= physunmap,
 	.read		= it8716f_spi_chip_read,
 	.write_256	= it8716f_spi_chip_write_256,
 	.write_aai	= spi_chip_write_1,
diff --git a/parallel.c b/parallel.c
index 9001fe80..e635088a 100644
--- a/parallel.c
+++ b/parallel.c
@@ -76,6 +76,17 @@ int register_par_master(const struct par_master *mst,
 		}
 	}
 
+	/* Bus masters supporting FWH/LPC cannot use fallback_map(), distinct
+	 * mappings are needed to support chips with FEATURE_REGISTERMAP
+	 */
+	if ((buses & (BUS_FWH | BUS_LPC)) && !mst->map_flash_region) {
+		msg_perr("%s called with incomplete master definition. "
+			 "FWH/LPC masters must provide memory mappings. "
+			 "Please report a bug at flashrom@flashrom.org\n",
+			 __func__);
+		return ERROR_FLASHROM_BUG;
+	}
+
 	if (!mst->chip_writeb || !mst->chip_writew || !mst->chip_writel ||
 	    !mst->chip_writen || !mst->chip_readb || !mst->chip_readw ||
 	    !mst->chip_readl || !mst->chip_readn) {
diff --git a/programmer.c b/programmer.c
index 2b960e6a..939d8c2d 100644
--- a/programmer.c
+++ b/programmer.c
@@ -20,7 +20,12 @@
 /* Fallback map() for programmers which don't need special handling */
 void *fallback_map(const char *descr, uintptr_t phys_addr, size_t len)
 {
-	/* FIXME: Should return phys_addr. */
+	/* A result of NULL causes mapped addresses to be chip physical
+	 * addresses, assuming only a single region is mapped (the entire flash
+	 * space).  Chips with a second region (like a register map) require a
+	 * real memory mapping to distinguish the different ranges.  Those chips
+	 * are FWH/LPC, so the bus master provides a real mapping.
+	 */
 	return NULL;
 }
 
diff --git a/sb600spi.c b/sb600spi.c
index 8b02b51d..6020145b 100644
--- a/sb600spi.c
+++ b/sb600spi.c
@@ -600,6 +600,8 @@ static const struct spi_master spi_master_sb600 = {
 	.max_data_write	= FIFO_SIZE_OLD - 3,
 	.command	= sb600_spi_send_command,
 	.multicommand	= default_spi_send_multicommand,
+	.map_flash_region	= physmap,
+	.unmap_flash_region	= physunmap,
 	.read		= default_spi_read,
 	.write_256	= default_spi_write_256,
 	.write_aai	= default_spi_write_aai,
@@ -612,6 +614,8 @@ static const struct spi_master spi_master_yangtze = {
 	.max_data_write	= FIFO_SIZE_YANGTZE - 3,
 	.command	= spi100_spi_send_command,
 	.multicommand	= default_spi_send_multicommand,
+	.map_flash_region	= physmap,
+	.unmap_flash_region	= physunmap,
 	.read		= default_spi_read,
 	.write_256	= default_spi_write_256,
 	.write_aai	= default_spi_write_aai,
@@ -624,6 +628,8 @@ static const struct spi_master spi_master_promontory = {
 	.max_data_write	= FIFO_SIZE_YANGTZE - 3,
 	.command	= spi100_spi_send_command,
 	.multicommand	= default_spi_send_multicommand,
+	.map_flash_region	= physmap,
+	.unmap_flash_region	= physunmap,
 	.read		= promontory_read_memmapped,
 	.write_256	= default_spi_write_256,
 	.write_aai	= default_spi_write_aai,
diff --git a/serprog.c b/serprog.c
index 12d15d74..a3a3db3e 100644
--- a/serprog.c
+++ b/serprog.c
@@ -438,7 +438,23 @@ static int serprog_shutdown(void *data)
 	return 0;
 }
 
+static void *serprog_map(const char *descr, uintptr_t phys_addr, size_t len)
+{
+	/* Serprog transmits 24 bits only and assumes the underlying implementation handles any remaining bits
+	 * correctly (usually setting them to one either in software (for FWH/LPC) or relying on the fact that
+	 * the hardware observes a subset of the address bits only). Combined with the standard mapping of
+	 * flashrom this creates a 16 MB-wide window just below the 4 GB boundary where serprog can operate (as
+	 * needed for non-SPI chips). Below we make sure that the requested range is within this window. */
+	if ((phys_addr & 0xFF000000) == 0xFF000000) {
+		return (void*)phys_addr;
+	}
+	msg_pwarn(MSGHEADER "requested mapping %s is incompatible: 0x%zx bytes at 0x%0*" PRIxPTR ".\n",
+		  descr, len, PRIxPTR_WIDTH, phys_addr);
+	return NULL;
+}
+
 static struct spi_master spi_master_serprog = {
+	.map_flash_region	= serprog_map,
 	.features	= SPI_MASTER_4BA,
 	.max_data_read	= MAX_DATA_READ_UNLIMITED,
 	.max_data_write	= MAX_DATA_WRITE_UNLIMITED,
@@ -553,6 +569,7 @@ static void serprog_chip_readn(const struct flashctx *flash, uint8_t * buf,
 }
 
 static const struct par_master par_master_serprog = {
+	.map_flash_region	= serprog_map,
 	.chip_readb	= serprog_chip_readb,
 	.chip_readw	= fallback_chip_readw,
 	.chip_readl	= fallback_chip_readl,
@@ -945,27 +962,11 @@ static void serprog_delay(unsigned int usecs)
 	sp_prev_was_write = 0;
 }
 
-static void *serprog_map(const char *descr, uintptr_t phys_addr, size_t len)
-{
-	/* Serprog transmits 24 bits only and assumes the underlying implementation handles any remaining bits
-	 * correctly (usually setting them to one either in software (for FWH/LPC) or relying on the fact that
-	 * the hardware observes a subset of the address bits only). Combined with the standard mapping of
-	 * flashrom this creates a 16 MB-wide window just below the 4 GB boundary where serprog can operate (as
-	 * needed for non-SPI chips). Below we make sure that the requested range is within this window. */
-	if ((phys_addr & 0xFF000000) == 0xFF000000) {
-		return (void*)phys_addr;
-	}
-	msg_pwarn(MSGHEADER "requested mapping %s is incompatible: 0x%zx bytes at 0x%0*" PRIxPTR ".\n",
-		  descr, len, PRIxPTR_WIDTH, phys_addr);
-	return NULL;
-}
-
 const struct programmer_entry programmer_serprog = {
 	.name			= "serprog",
 	.type			= OTHER,
 				/* FIXME */
 	.devs.note		= "All programmer devices speaking the serprog protocol\n",
 	.init			= serprog_init,
-	.map_flash_region	= serprog_map,
 	.delay			= serprog_delay,
 };
diff --git a/wbsio_spi.c b/wbsio_spi.c
index 95ca1e99..661ecbb1 100644
--- a/wbsio_spi.c
+++ b/wbsio_spi.c
@@ -187,6 +187,8 @@ static const struct spi_master spi_master_wbsio = {
 	.max_data_write	= MAX_DATA_UNSPECIFIED,
 	.command	= wbsio_spi_send_command,
 	.multicommand	= default_spi_send_multicommand,
+	.map_flash_region	= physmap,
+	.unmap_flash_region	= physunmap,
 	.read		= wbsio_spi_read,
 	.write_256	= spi_chip_write_1,
 	.write_aai	= spi_chip_write_1,