Patchwork internal programmer: handle failed chip mapping

login
register
about
Submitter Carl-Daniel Hailfinger
Date 2014-03-04 01:30:03
Message ID <53152C9B.4030702@gmx.net>
Download mbox | patch
Permalink /patch/4114/
State New
Headers show

Comments

Carl-Daniel Hailfinger - 2014-03-04 01:30:03
Mapping a chip with the internal programmer on x86 will fail if the chip
is larger than 16 MByte due to traditional x86 architecture constraints.
No LPC/FWH chips of sizes bigger than 4 MByte are known, and Parallel
chips are limited to older x86 chipsets and way smaller sizes because of
address line shortage. We do see quite large SPI chips (even >16 MByte)
and flashrom shouldn't fail probing or accessing them. Given that we
have no chance to map those chips into the address space completely,
make sure they are handled properly with indirect accesses

Caveat: The code is proof-of-concept quality and litters the codebase
with exit(1). We need to discuss what do do in that case.

Testing should be done on a x86 machine which has SPI flash, and with
r1702 reverted to have the 32 MByte chips again in flashchips.c (ignore
the incorrect r1701 changelog which speaks of 32 MByte chip removal,
that only happens in r1702).

If flashrom successfully finishes probing without an abort in between,
the patch does what it should. Or not. Read on.
Without this patch and a reverted r1702, flashrom may successfully
finish probing anyway due to the earlier physmap cleanups by Niklas
Söderlund (r1744 and r1745). That's just the result from missing error
checks, though.
Applying this patch against a r1701 checkout (all files being r1701)
should result in a fixed probe vs. a failing probe for plain r1701.

A backported version of the patch against r1701 can be found at
http://paste.flashrom.org/view.php?id=2027

Thanks for reading.

Regards,
Carl-Daniel

Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net>

Patch

Index: flashrom-internal_no_chip_mapping_larger_16mbyte/flash.h
===================================================================
--- flashrom-internal_no_chip_mapping_larger_16mbyte/flash.h	(Revision 1764)
+++ flashrom-internal_no_chip_mapping_larger_16mbyte/flash.h	(Arbeitskopie)
@@ -186,6 +186,7 @@ 
 
 struct flashctx {
 	struct flashchip *chip;
+	bool virtual_memory_is_usable;
 	chipaddr virtual_memory;
 	/* Some flash devices have an additional register space. */
 	chipaddr virtual_registers;
Index: flashrom-internal_no_chip_mapping_larger_16mbyte/it87spi.c
===================================================================
--- flashrom-internal_no_chip_mapping_larger_16mbyte/it87spi.c	(Revision 1764)
+++ flashrom-internal_no_chip_mapping_larger_16mbyte/it87spi.c	(Arbeitskopie)
@@ -385,7 +385,7 @@ 
 	 * the mainboard does not use IT87 SPI translation. This should be done
 	 * via a programmer parameter for the internal programmer.
 	 */
-	if ((flash->chip->total_size * 1024 > 512 * 1024)) {
+	if (!flash->virtual_memory_is_usable || (flash->chip->total_size * 1024 > 512 * 1024)) {
 		spi_read_chunked(flash, buf, start, len, 3);
 	} else {
 		mmio_readn((void *)(flash->virtual_memory + start), buf, len);
Index: flashrom-internal_no_chip_mapping_larger_16mbyte/internal.c
===================================================================
--- flashrom-internal_no_chip_mapping_larger_16mbyte/internal.c	(Revision 1764)
+++ flashrom-internal_no_chip_mapping_larger_16mbyte/internal.c	(Arbeitskopie)
@@ -365,45 +365,69 @@ 
 }
 #endif
 
+void *internal_map(const char *descr, uintptr_t phys_addr, size_t len)
+{
+	return physmap(descr, phys_addr, len);
+}
+
+void internal_unmap(void *virt_addr, size_t len)
+{
+	physunmap(virt_addr, len);
+}
+
 static void internal_chip_writeb(const struct flashctx *flash, uint8_t val,
 				 chipaddr addr)
 {
+	if (!flash->virtual_memory_is_usable)
+		exit(1);
 	mmio_writeb(val, (void *) addr);
 }
 
 static void internal_chip_writew(const struct flashctx *flash, uint16_t val,
 				 chipaddr addr)
 {
+	if (!flash->virtual_memory_is_usable)
+		exit(1);
 	mmio_writew(val, (void *) addr);
 }
 
 static void internal_chip_writel(const struct flashctx *flash, uint32_t val,
 				 chipaddr addr)
 {
+	if (!flash->virtual_memory_is_usable)
+		exit(1);
 	mmio_writel(val, (void *) addr);
 }
 
 static uint8_t internal_chip_readb(const struct flashctx *flash,
 				   const chipaddr addr)
 {
+	if (!flash->virtual_memory_is_usable)
+		exit(1);
 	return mmio_readb((void *) addr);
 }
 
 static uint16_t internal_chip_readw(const struct flashctx *flash,
 				    const chipaddr addr)
 {
+	if (!flash->virtual_memory_is_usable)
+		exit(1);
 	return mmio_readw((void *) addr);
 }
 
 static uint32_t internal_chip_readl(const struct flashctx *flash,
 				    const chipaddr addr)
 {
+	if (!flash->virtual_memory_is_usable)
+		exit(1);
 	return mmio_readl((void *) addr);
 }
 
 static void internal_chip_readn(const struct flashctx *flash, uint8_t *buf,
 				const chipaddr addr, size_t len)
 {
+	if (!flash->virtual_memory_is_usable)
+		exit(1);
 	mmio_readn((void *)addr, buf, len);
 	return;
 }
Index: flashrom-internal_no_chip_mapping_larger_16mbyte/wbsio_spi.c
===================================================================
--- flashrom-internal_no_chip_mapping_larger_16mbyte/wbsio_spi.c	(Revision 1764)
+++ flashrom-internal_no_chip_mapping_larger_16mbyte/wbsio_spi.c	(Arbeitskopie)
@@ -20,6 +20,7 @@ 
 
 #if defined(__i386__) || defined(__x86_64__)
 
+#include <stdlib.h>
 #include "flash.h"
 #include "chipdrivers.h"
 #include "programmer.h"
@@ -204,6 +205,8 @@ 
 static int wbsio_spi_read(struct flashctx *flash, uint8_t *buf,
 			  unsigned int start, unsigned int len)
 {
+	if (!flash->virtual_memory_is_usable)
+		exit(1);
 	mmio_readn((void *)(flash->virtual_memory + start), buf, len);
 	return 0;
 }
Index: flashrom-internal_no_chip_mapping_larger_16mbyte/flashrom.c
===================================================================
--- flashrom-internal_no_chip_mapping_larger_16mbyte/flashrom.c	(Revision 1764)
+++ flashrom-internal_no_chip_mapping_larger_16mbyte/flashrom.c	(Arbeitskopie)
@@ -68,8 +68,8 @@ 
 		.type			= OTHER,
 		.devs.note		= NULL,
 		.init			= internal_init,
-		.map_flash_region	= physmap,
-		.unmap_flash_region	= physunmap,
+		.map_flash_region	= internal_map,
+		.unmap_flash_region	= internal_unmap,
 		.delay			= internal_delay,
 	},
 #endif
@@ -1086,6 +1086,10 @@ 
 
 		base = flashbase ? flashbase : (0xffffffff - size + 1);
 		flash->virtual_memory = (chipaddr)programmer_map_flash_region("flash chip", base, size);
+		if (flash->virtual_memory == (chipaddr)ERROR_PTR)
+			flash->virtual_memory_is_usable = false;
+		else
+			flash->virtual_memory_is_usable = true;
 
 		/* We handle a forced match like a real match, we just avoid probing. Note that probe_flash()
 		 * is only called with force=1 after normal probing failed.
Index: flashrom-internal_no_chip_mapping_larger_16mbyte/programmer.h
===================================================================
--- flashrom-internal_no_chip_mapping_larger_16mbyte/programmer.h	(Revision 1764)
+++ flashrom-internal_no_chip_mapping_larger_16mbyte/programmer.h	(Arbeitskopie)
@@ -325,6 +325,8 @@ 
 int register_superio(struct superio s);
 extern enum chipbustype internal_buses_supported;
 int internal_init(void);
+void *internal_map(const char *descr, uintptr_t phys_addr, size_t len);
+void internal_unmap(void *virt_addr, size_t len);
 #endif
 
 /* hwaccess.c */