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>

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,
 };

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,
 };

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.

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,

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]))

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);

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,
 };

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,

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) {

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;
 }
 

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,

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,
 };

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,