[flashrom] [PATCH 1/2] Error out when chipset/board doesn't decode full ROM

Carl-Daniel Hailfinger c-d.hailfinger.devel.2006 at gmx.net
Sat Oct 31 02:52:12 CET 2009


On 31.10.2009 01:01, Uwe Hermann wrote:
> On Fri, Oct 30, 2009 at 01:52:28AM +0100, Carl-Daniel Hailfinger wrote:
>   
>> Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net>
>>     
>
> The slightly updated version attached to this mail is
>
> Acked-by: Uwe Hermann <uwe at hermann-uwe.de>
>   

Thanks!


> I tested this on an FWH board, one parallel board, and one nic3com with
> parallel flash.
>
> Changes in the updated patch:
>
>  - Running "flashrom" without options also shows the error if the chip
>    is too big (not just -r, -E, or -w).
>
>  - Print ROM sizes in KB (not bytes) to be more user-friendly and better
>    readable.
>
>  - Make the error that gets printed without -V fit into one line:
>    "Chip is too big for this programmer (-V gives details). Use --force to override."
>    Also, add missing \n to that string.
>   

I agree with the changes. On top of your changes, I fixed one memory
leak and changed kB vs. KB (for consistency), patch is below for reference.

Add infrastructure to check and report to the user the maximum supported
decode size for chipsets and tested mainboards.

The rationale is to warn users when they, for example, try to flash
a 512KB parallel flash chip but their chipset only supports 256KB,
or they try to flash 512KB and the chipset _does_ theoretically
support 512KB but their special board doesn't wire all address lines
and thus supports only 256 KB ROM chips at maximum.

This has cost Uwe hours of debugging on some board already, until he
figured out what was going on. We should try warn our users where
possible about this.

The chipset and the chip may have more than one bus in common (e.g.
SB600 and Pm49* can both speak LPC+FWH) and on SB600/SB7x0/SB8x0 there
are different limits for LPC and FWH. The only way to tell the user
about the exact circumstances is to spew error messages per bus.

The code will issue a warning during probe (which does fail for some
chips if the size is too big) and abort before the first real
read/write/erase action. If no action is specified, the warning is
printed anyway.
That way, a user can find out why probe might not have worked, and will
be stopped before he/she gets incorrect results.

Add a bitcount function to the infrastructure.

Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net>
Acked-by: Uwe Hermann <uwe at hermann-uwe.de>

Index: flashrom-rom_decode_check_infrastructure/flash.h
===================================================================
--- flashrom-rom_decode_check_infrastructure/flash.h	(Revision 753)
+++ flashrom-rom_decode_check_infrastructure/flash.h	(Arbeitskopie)
@@ -352,9 +352,17 @@
 void sio_mask(uint16_t port, uint8_t reg, uint8_t data, uint8_t mask);
 int board_flash_enable(const char *vendor, const char *part);
 
+struct decode_sizes {
+	uint32_t parallel;
+	uint32_t lpc;
+	uint32_t fwh;
+	uint32_t spi;
+};
+
 /* chipset_enable.c */
 extern enum chipbustype buses_supported;
 int chipset_flash_enable(void);
+extern struct decode_sizes max_rom_decode;
 
 extern unsigned long flashbase;
 
Index: flashrom-rom_decode_check_infrastructure/chipset_enable.c
===================================================================
--- flashrom-rom_decode_check_infrastructure/chipset_enable.c	(Revision 753)
+++ flashrom-rom_decode_check_infrastructure/chipset_enable.c	(Arbeitskopie)
@@ -42,6 +42,17 @@
 
 enum chipbustype buses_supported = CHIP_BUSTYPE_NONSPI;
 
+/**
+ * Programmers supporting multiple buses can have differing size limits on
+ * each bus. Store the limits for each bus in a common struct.
+ */
+struct decode_sizes max_rom_decode = {
+	.parallel	= 0xffffffff,
+	.lpc		= 0xffffffff,
+	.fwh		= 0xffffffff,
+	.spi		= 0xffffffff
+};
+
 extern int ichspi_lock;
 
 static int enable_flash_ali_m1533(struct pci_dev *dev, const char *name)
Index: flashrom-rom_decode_check_infrastructure/flashrom.c
===================================================================
--- flashrom-rom_decode_check_infrastructure/flashrom.c	(Revision 753)
+++ flashrom-rom_decode_check_infrastructure/flashrom.c	(Arbeitskopie)
@@ -299,6 +299,15 @@
 	return (a > b) ? a : b;
 }
 
+int bitcount(unsigned long a)
+{
+	int i = 0;
+	for (; a != 0; a >>= 1)
+		if (a & 1)
+			i++;
+	return i;
+}
+
 char *strcat_realloc(char *dest, const char *src)
 {
 	dest = realloc(dest, strlen(dest) + strlen(src) + 1);
@@ -398,10 +407,60 @@
 	return ret;
 }
 
+int check_max_decode(enum chipbustype buses, uint32_t size)
+{
+	int limitexceeded = 0;
+	if ((buses & CHIP_BUSTYPE_PARALLEL) &&
+	    (max_rom_decode.parallel < size)) {
+		limitexceeded++;
+		printf_debug("Chip size %u kB is bigger than supported "
+			     "size %u kB of chipset/board/programmer "
+			     "for %s interface, "
+			     "probe/read/erase/write may fail. ", size / 1024,
+			     max_rom_decode.parallel / 1024, "Parallel");
+	}
+	if ((buses & CHIP_BUSTYPE_LPC) && (max_rom_decode.lpc < size)) {
+		limitexceeded++;
+		printf_debug("Chip size %u kB is bigger than supported "
+			     "size %u kB of chipset/board/programmer "
+			     "for %s interface, "
+			     "probe/read/erase/write may fail. ", size / 1024,
+			     max_rom_decode.lpc / 1024, "LPC");
+	}
+	if ((buses & CHIP_BUSTYPE_FWH) && (max_rom_decode.fwh < size)) {
+		limitexceeded++;
+		printf_debug("Chip size %u kB is bigger than supported "
+			     "size %u kB of chipset/board/programmer "
+			     "for %s interface, "
+			     "probe/read/erase/write may fail. ", size / 1024,
+			     max_rom_decode.fwh / 1024, "FWH");
+	}
+	if ((buses & CHIP_BUSTYPE_SPI) && (max_rom_decode.spi < size)) {
+		limitexceeded++;
+		printf_debug("Chip size %u kB is bigger than supported "
+			     "size %u kB of chipset/board/programmer "
+			     "for %s interface, "
+			     "probe/read/erase/write may fail. ", size / 1024,
+			     max_rom_decode.spi / 1024, "SPI");
+	}
+	if (!limitexceeded)
+		return 0;
+	/* Sometimes chip and programmer have more than one bus in common,
+	 * and the limit is not exceeded on all buses. Tell the user.
+	 */
+	if (bitcount(buses) > limitexceeded)
+		printf_debug("There is at least one common chip/programmer "
+			     "interface which can support a chip of this size. "
+			     "You can try --force at your own risk.\n");
+	return 1;
+}
+
 struct flashchip *probe_flash(struct flashchip *first_flash, int force)
 {
 	struct flashchip *flash;
-	unsigned long base = 0, size;
+	unsigned long base = 0;
+	uint32_t size;
+	enum chipbustype buses_common;
 	char *tmp;
 
 	for (flash = first_flash; flash && flash->name; flash++) {
@@ -413,7 +472,8 @@
 			printf_debug("failed! flashrom has no probe function for this flash chip.\n");
 			continue;
 		}
-		if (!(buses_supported & flash->bustype)) {
+		buses_common = buses_supported & flash->bustype;
+		if (!buses_common) {
 			tmp = flashbuses_to_text(buses_supported);
 			printf_debug("skipped. Host bus type %s ", tmp);
 			free(tmp);
@@ -424,6 +484,7 @@
 		}
 
 		size = flash->total_size * 1024;
+		check_max_decode(buses_common, size);
 
 		base = flashbase ? flashbase : (0xffffffff - size + 1);
 		flash->virtual_memory = (chipaddr)programmer_map_flash_region("flash chip", base, size);
@@ -956,6 +1017,15 @@
 		       "mainboard you tested. Thanks for your help!\n===\n");
 	}
 
+	size = flash->total_size * 1024;
+	if (check_max_decode((buses_supported & flash->bustype), size) &&
+	    (!force)) {
+		fprintf(stderr, "Chip is too big for this programmer "
+			"(-V gives details). Use --force to override.\n");
+		programmer_shutdown();
+		return 1;
+	}
+
 	if (!(read_it | write_it | verify_it | erase_it)) {
 		printf("No operations were specified.\n");
 		// FIXME: flash writes stay enabled!
@@ -974,7 +1044,6 @@
 	if (write_it && !dont_verify_it)
 		verify_it = 1;
 
-	size = flash->total_size * 1024;
 	buf = (uint8_t *) calloc(size, sizeof(char));
 
 	if (erase_it) {


-- 
Developer quote of the week: 
"We are juggling too many chainsaws and flaming arrows and tigers."





More information about the flashrom mailing list