Automatically unmap physmap()s

Similarly to the previous PCI self-clean up patch this one allows to get rid
of a huge number of programmer shutdown functions and makes introducing
bugs harder. It adds a new function rphysmap() that takes care of unmapping
at shutdown. Callers are changed where it makes sense.

Corresponding to flashrom svn r1714.

Signed-off-by: Stefan Tauner <stefan.tauner@alumni.tuwien.ac.at>
Acked-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net>

chipset_enable.c

@@ -480,7 +480,9 @@ static int enable_flash_tunnelcreek(struct pci_dev *dev, const char *name)
 	msg_pdbg("\nRoot Complex Register Block address = 0x%x\n", tmp);
 
 	/* Map RCBA to virtual memory */
-	rcrb = physmap("ICH RCRB", tmp, 0x4000);
+	rcrb = rphysmap("ICH RCRB", tmp, 0x4000);
+	if (rcrb == ERROR_PTR)
+		return 1;
 
 	/* Test Boot BIOS Strap Status */
 	bnt = mmio_readl(rcrb + 0x3410);
@@ -562,7 +564,9 @@ static int enable_flash_ich_dc_spi(struct pci_dev *dev, const char *name,
 	msg_pdbg("Root Complex Register Block address = 0x%x\n", tmp);
 
 	/* Map RCBA to virtual memory */
-	rcrb = physmap("ICH RCRB", tmp, 0x4000);
+	rcrb = rphysmap("ICH RCRB", tmp, 0x4000);
+	if (rcrb == ERROR_PTR)
+		return 1;
 
 	gcs = mmio_readl(rcrb + 0x3410);
 	msg_pdbg("GCS = 0x%x: ", gcs);
@@ -737,10 +741,8 @@ static int enable_flash_vt_vx(struct pci_dev *dev, const char *name)
 		case 0x8410: /* VX900 */
 			mmio_base = pci_read_long(dev, 0xbc) << 8;
 			mmio_base_physmapped = physmap("VIA VX MMIO register", mmio_base, SPI_CNTL_LEN);
-			if (mmio_base_physmapped == ERROR_PTR) {
+			if (mmio_base_physmapped == ERROR_PTR)
-				physunmap(mmio_base_physmapped, SPI_CNTL_LEN);
 				return ERROR_FATAL;
-			}
 
 			/* Offset 0 - Bit 0 holds SPI Bus0 Enable Bit. */
 			spi_cntl = mmio_readl(mmio_base_physmapped) + 0x00;

drkaiser.c

@@ -56,12 +56,6 @@ static const struct par_programmer par_programmer_drkaiser = {
 		.chip_writen		= fallback_chip_writen,
 };
 
-static int drkaiser_shutdown(void *data)
-{
-	physunmap(drkaiser_bar, DRKAISER_MEMMAP_SIZE);
-	return 0;
-}
-
 int drkaiser_init(void)
 {
 	struct pci_dev *dev = NULL;
@@ -80,10 +74,8 @@ int drkaiser_init(void)
 	rpci_write_word(dev, PCI_MAGIC_DRKAISER_ADDR, PCI_MAGIC_DRKAISER_VALUE);
 
 	/* Map 128kB flash memory window. */
-	drkaiser_bar = physmap("Dr. Kaiser PC-Waechter flash memory",
+	drkaiser_bar = rphysmap("Dr. Kaiser PC-Waechter flash memory", addr, DRKAISER_MEMMAP_SIZE);
-			       addr, DRKAISER_MEMMAP_SIZE);
+	if (drkaiser_bar == ERROR_PTR)
-
-	if (register_shutdown(drkaiser_shutdown, NULL))
 		return 1;
 
 	max_rom_decode.parallel = 128 * 1024;

gfxnvidia.c

@@ -77,12 +77,6 @@ static const struct par_programmer par_programmer_gfxnvidia = {
 		.chip_writen		= fallback_chip_writen,
 };
 
-static int gfxnvidia_shutdown(void *data)
-{
-	physunmap(nvidia_bar, GFXNVIDIA_MEMMAP_SIZE);
-	return 0;
-}
-
 int gfxnvidia_init(void)
 {
 	struct pci_dev *dev = NULL;
@@ -99,9 +93,8 @@ int gfxnvidia_init(void)
 	io_base_addr += 0x300000;
 	msg_pinfo("Detected NVIDIA I/O base address: 0x%x.\n", io_base_addr);
 
-	nvidia_bar = physmap("NVIDIA", io_base_addr, GFXNVIDIA_MEMMAP_SIZE);
+	nvidia_bar = rphysmap("NVIDIA", io_base_addr, GFXNVIDIA_MEMMAP_SIZE);
-
+	if (nvidia_bar == ERROR_PTR)
-	if (register_shutdown(gfxnvidia_shutdown, NULL))
 		return 1;
 
 	/* Allow access to flash interface (will disable screen). */

ichspi.c

@@ -1844,7 +1844,9 @@ int via_init_spi(struct pci_dev *dev, uint32_t mmio_base)
 {
 	int i;
 
-	ich_spibar = physmap("VIA SPI MMIO registers", mmio_base, 0x70);
+	ich_spibar = rphysmap("VIA SPI MMIO registers", mmio_base, 0x70);
+	if (ich_spibar == ERROR_PTR)
+		return ERROR_FATAL;
 	/* Do we really need no write enable? Like the LPC one at D17F0 0x40 */
 
 	/* Not sure if it speaks all these bus protocols. */

it85spi.c

@@ -262,6 +262,9 @@ static int it85xx_spi_common_init(struct superio s)
 	 * Major TODO here, and it will be a lot of work.
 	 */
 	base = (chipaddr)physmap("it85 communication", 0xFFFFF000, 0x1000);
+	if (base == (chipaddr)ERROR_PTR)
+		return 1;
+
 	msg_pdbg("%s():%d base=0x%08x\n", __func__, __LINE__,
 	         (unsigned int)base);
 	ce_high = (unsigned char *)(base + 0xE00);  /* 0xFFFFFE00 */

mcp6x_spi.c

@@ -135,25 +135,20 @@ int mcp6x_spi_init(int want_spi)
 
 	/* Accessing a NULL pointer BAR is evil. Don't do it. */
 	if (!mcp6x_spibaraddr && want_spi) {
-		msg_perr("Error: Chipset is strapped for SPI, but MCP SPI BAR "
+		msg_perr("Error: Chipset is strapped for SPI, but MCP SPI BAR is invalid.\n");
-			 "is invalid.\n");
 		return 1;
 	} else if (!mcp6x_spibaraddr && !want_spi) {
 		msg_pdbg("MCP SPI is not used.\n");
 		return 0;
 	} else if (mcp6x_spibaraddr && !want_spi) {
-		msg_pdbg("Strange. MCP SPI BAR is valid, but chipset apparently"
+		msg_pdbg("Strange. MCP SPI BAR is valid, but chipset apparently doesn't have SPI enabled.\n");
-			 " doesn't have SPI enabled.\n");
 		/* FIXME: Should we enable SPI anyway? */
 		return 0;
 	}
 	/* Map the BAR. Bytewise/wordwise access at 0x530 and 0x540. */
-	mcp6x_spibar = physmap("NVIDIA MCP6x SPI", mcp6x_spibaraddr, 0x544);
+	mcp6x_spibar = rphysmap("NVIDIA MCP6x SPI", mcp6x_spibaraddr, 0x544);
-
+	if (mcp6x_spibar == ERROR_PTR)
-#if 0
+		return 1;
-	/* FIXME: Run the physunmap in a shutdown function. */
-	physunmap(mcp6x_spibar, 0x544);
-#endif
 
 	status = mmio_readw(mcp6x_spibar + 0x530);
 	msg_pdbg("SPI control is 0x%04x, req=%i, gnt=%i\n",

nicintel.c

@@ -59,13 +59,6 @@ static const struct par_programmer par_programmer_nicintel = {
 		.chip_writen		= fallback_chip_writen,
 };
 
-static int nicintel_shutdown(void *data)
-{
-	physunmap(nicintel_control_bar, NICINTEL_CONTROL_MEMMAP_SIZE);
-	physunmap(nicintel_bar, NICINTEL_MEMMAP_SIZE);
-	return 0;
-}
-
 int nicintel_init(void)
 {
 	struct pci_dev *dev = NULL;
@@ -83,18 +76,14 @@ int nicintel_init(void)
 		return 1;
 
 	addr = pcidev_readbar(dev, PCI_BASE_ADDRESS_2);
-	nicintel_bar = physmap("Intel NIC flash", addr, NICINTEL_MEMMAP_SIZE);
+	nicintel_bar = rphysmap("Intel NIC flash", addr, NICINTEL_MEMMAP_SIZE);
 	if (nicintel_bar == ERROR_PTR)
-		goto error_out_unmap;
+		return 1;
 
 	addr = pcidev_readbar(dev, PCI_BASE_ADDRESS_0);
 	/* FIXME: This is not an aligned mapping. Use 4k? */
-	nicintel_control_bar = physmap("Intel NIC control/status reg",
+	nicintel_control_bar = rphysmap("Intel NIC control/status reg", addr, NICINTEL_CONTROL_MEMMAP_SIZE);
-	                               addr, NICINTEL_CONTROL_MEMMAP_SIZE);
 	if (nicintel_control_bar == ERROR_PTR)
-		goto error_out;
-
-	if (register_shutdown(nicintel_shutdown, NULL))
 		return 1;
 
 	/* FIXME: This register is pretty undocumented in all publicly available
@@ -112,11 +101,6 @@ int nicintel_init(void)
 	register_par_programmer(&par_programmer_nicintel, BUS_PARALLEL);
 
 	return 0;
-
-error_out_unmap:
-	physunmap(nicintel_bar, NICINTEL_MEMMAP_SIZE);
-error_out:
-	return 1;
 }
 
 static void nicintel_chip_writeb(const struct flashctx *flash, uint8_t val,

nicintel_spi.c

@@ -151,16 +151,12 @@ static int nicintel_spi_shutdown(void *data)
 {
 	uint32_t tmp;
 
-	/* Disable writes manually. See the comment about EECD in
+	/* Disable writes manually. See the comment about EECD in nicintel_spi_init() for details. */
-	 * nicintel_spi_init() for details.
-	 */
 	tmp = pci_mmio_readl(nicintel_spibar + EECD);
 	tmp &= ~FLASH_WRITES_ENABLED;
 	tmp |= FLASH_WRITES_DISABLED;
 	pci_mmio_writel(tmp, nicintel_spibar + EECD);
 
-	physunmap(nicintel_spibar, MEMMAP_SIZE);
-
 	return 0;
 }
 
@@ -177,8 +173,7 @@ int nicintel_spi_init(void)
 		return 1;
 
 	io_base_addr = pcidev_readbar(dev, PCI_BASE_ADDRESS_0);
-	nicintel_spibar = physmap("Intel Gigabit NIC w/ SPI flash",
+	nicintel_spibar = rphysmap("Intel Gigabit NIC w/ SPI flash", io_base_addr, MEMMAP_SIZE);
-				  io_base_addr, MEMMAP_SIZE);
 	/* Automatic restore of EECD on shutdown is not possible because EECD
 	 * does not only contain FLASH_WRITES_DISABLED|FLASH_WRITES_ENABLED,
 	 * but other bits with side effects as well. Those other bits must be

ogp_spi.c

@@ -97,12 +97,6 @@ static const struct bitbang_spi_master bitbang_spi_master_ogp = {
 	.half_period = 0,
 };
 
-static int ogp_spi_shutdown(void *data)
-{
-	physunmap(ogp_spibar, 4096);
-	return 0;
-}
-
 int ogp_spi_init(void)
 {
 	struct pci_dev *dev = NULL;
@@ -137,9 +131,8 @@ int ogp_spi_init(void)
 		return 1;
 
 	io_base_addr = pcidev_readbar(dev, PCI_BASE_ADDRESS_0);
-	ogp_spibar = physmap("OGP registers", io_base_addr, 4096);
+	ogp_spibar = rphysmap("OGP registers", io_base_addr, 4096);
-
+	if (ogp_spibar == ERROR_PTR)
-	if (register_shutdown(ogp_spi_shutdown, NULL))
 		return 1;
 
 	if (bitbang_spi_init(&bitbang_spi_master_ogp))

physmap.c

@@ -21,6 +21,7 @@
  */
 
 #include <unistd.h>
+#include <stdbool.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
@@ -204,13 +205,32 @@ void physunmap(void *virt_addr, size_t len)
 }
 #endif
 
-#define PHYSMAP_NOFAIL	0
+#define PHYSMAP_NOFAIL		0
-#define PHYSMAP_MAYFAIL	1
+#define PHYSMAP_MAYFAIL		1
-#define PHYSMAP_RW	0
+#define PHYSMAP_RW		0
-#define PHYSMAP_RO	1
+#define PHYSMAP_RO		1
+#define PHYSMAP_NOCLEANUP	0
+#define PHYSMAP_CLEANUP		1
 
-static void *physmap_common(const char *descr, uintptr_t phys_addr,
+struct undo_physmap_data {
-			    size_t len, int mayfail, int readonly)
+	void *virt_addr;
+	size_t len;
+};
+
+static int undo_physmap(void *data)
+{
+	if (data == NULL) {
+		msg_perr("%s: tried to physunmap without valid data!\n", __func__);
+		return 1;
+	}
+	struct undo_physmap_data *d = data;
+	physunmap(d->virt_addr, d->len);
+	free(data);
+	return 0;
+}
+
+static void *physmap_common(const char *descr, uintptr_t phys_addr, size_t len, bool mayfail,
+			    bool readonly, bool autocleanup)
 {
 	void *virt_addr;
 
@@ -257,19 +277,42 @@ static void *physmap_common(const char *descr, uintptr_t phys_addr,
 			exit(3);
 	}
 
+	if (autocleanup) {
+		struct undo_physmap_data *d = malloc(sizeof(struct undo_physmap_data));
+		if (d == NULL) {
+			msg_perr("%s: Out of memory!\n", __func__);
+			goto unmap_out;
+		}
+
+		d->virt_addr = virt_addr;
+		d->len = len;
+		if (register_shutdown(undo_physmap, d) != 0) {
+			msg_perr("%s: Could not register shutdown function!\n", __func__);
+			goto unmap_out;
+		}
+	}
+
 	return virt_addr;
+unmap_out:
+	physunmap(virt_addr, len);
+	if (!mayfail)
+		exit(3);
+	return ERROR_PTR;
 }
 
 void *physmap(const char *descr, uintptr_t phys_addr, size_t len)
 {
-	return physmap_common(descr, phys_addr, len, PHYSMAP_NOFAIL,
+	return physmap_common(descr, phys_addr, len, PHYSMAP_NOFAIL, PHYSMAP_RW, PHYSMAP_NOCLEANUP);
-			      PHYSMAP_RW);
+}
+
+void *rphysmap(const char *descr, uintptr_t phys_addr, size_t len)
+{
+	return physmap_common(descr, phys_addr, len, PHYSMAP_NOFAIL, PHYSMAP_RW, PHYSMAP_CLEANUP);
 }
 
 void *physmap_try_ro(const char *descr, uintptr_t phys_addr, size_t len)
 {
-	return physmap_common(descr, phys_addr, len, PHYSMAP_MAYFAIL,
+	return physmap_common(descr, phys_addr, len, PHYSMAP_MAYFAIL, PHYSMAP_RO, PHYSMAP_NOCLEANUP);
-			      PHYSMAP_RO);
 }
 
 /* MSR abstraction implementations for Linux, OpenBSD, FreeBSD/Dragonfly, OSX, libpayload

programmer.h

@@ -276,6 +276,7 @@ int processor_flash_enable(void);
 
 /* physmap.c */
 void *physmap(const char *descr, uintptr_t phys_addr, size_t len);
+void *rphysmap(const char *descr, uintptr_t phys_addr, size_t len);
 void *physmap_try_ro(const char *descr, uintptr_t phys_addr, size_t len);
 void physunmap(void *virt_addr, size_t len);
 #if CONFIG_INTERNAL == 1

satamv.c

@@ -57,12 +57,6 @@ static const struct par_programmer par_programmer_satamv = {
 		.chip_writen		= fallback_chip_writen,
 };
 
-static int satamv_shutdown(void *data)
-{
-	physunmap(mv_bar, 0x20000);
-	return 0;
-}
-
 /*
  * Random notes:
  * FCE#		Flash Chip Enable
@@ -94,13 +88,10 @@ int satamv_init(void)
 		return 1;
 
 	addr = pcidev_readbar(dev, PCI_BASE_ADDRESS_0);
-	mv_bar = physmap("Marvell 88SX7042 registers", addr, 0x20000);
+	mv_bar = rphysmap("Marvell 88SX7042 registers", addr, 0x20000);
 	if (mv_bar == ERROR_PTR)
 		return 1;
 
-	if (register_shutdown(satamv_shutdown, NULL))
-		return 1;
-
 	tmp = pci_mmio_readl(mv_bar + FLASH_PARAM);
 	msg_pspew("Flash Parameters:\n");
 	msg_pspew("TurnOff=0x%01x\n", (tmp >> 0) & 0x7);

satasii.c

@@ -54,12 +54,6 @@ static const struct par_programmer par_programmer_satasii = {
 		.chip_writen		= fallback_chip_writen,
 };
 
-static int satasii_shutdown(void *data)
-{
-	physunmap(sii_bar, SATASII_MEMMAP_SIZE);
-	return 0;
-}
-
 static uint32_t satasii_wait_done(void)
 {
 	uint32_t ctrl_reg;
@@ -97,15 +91,15 @@ int satasii_init(void)
 		reg_offset = 0x50;
 	}
 
-	sii_bar = physmap("SATA SiI registers", addr, SATASII_MEMMAP_SIZE) + reg_offset;
+	sii_bar = rphysmap("SATA SiI registers", addr, SATASII_MEMMAP_SIZE);
+	if (sii_bar == ERROR_PTR)
+		return 1;
+	sii_bar += reg_offset;
 
 	/* Check if ROM cycle are OK. */
 	if ((id != 0x0680) && (!(pci_mmio_readl(sii_bar) & (1 << 26))))
 		msg_pwarn("Warning: Flash seems unconnected.\n");
 
-	if (register_shutdown(satasii_shutdown, NULL))
-		return 1;
-
 	register_par_programmer(&par_programmer_satasii, BUS_PARALLEL);
 
 	return 0;

sb600spi.c

@@ -326,8 +326,10 @@ int sb600_probe_spi(struct pci_dev *dev)
 		return 0;
 
 	/* Physical memory has to be mapped at page (4k) boundaries. */
-	sb600_spibar = physmap("SB600 SPI registers", tmp & 0xfffff000,
+	sb600_spibar = rphysmap("SB600 SPI registers", tmp & 0xfffff000, 0x1000);
-			       0x1000);
+	if (sb600_spibar == ERROR_PTR)
+		return 1;
+
 	/* The low bits of the SPI base address are used as offset into
 	 * the mapped page.
 	 */