[flashrom] [PATCH] dummy programmer: blacklist SPI commands

[Maciej Pijanka]

On Wed, 19 Jan 2011, Carl-Daniel Hailfinger wrote:

> New version with additional bugfixes and man page.
> The man page formatting is broken, I'd appreciate help from someone who
> knows how to fix the formatting.

What is wrong with manpage formatting, of course besides content, few parts could
use some work. 

> Add optional SPI command blacklisting to the flash chip emulator in the
> dummy programmer.
> 
> Usage:
> flashrom -p dummy:spi_blacklist=hexstring
> 
> If hexstring is 0302, flashrom will blacklist command 0x03 (READ) and
> command 0x02 (WRITE). The hexstring can have up to 521 bytes (256
> commands) length, and must not start with 0x.
> 
> TODO: Should 0x at the beginning of hexstring be tolerated?

Above remark seems to be inaccurate or outdated, as code skips 0x if its on
start of string.

> Very useful for testing corner cases if you don't own a locked down
> Intel chipset and want to simulate such a thing.
> 
> Fix out-of-bounds access in the simulator command checking code.
> 
> Signed-off-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006 at gmx.net>
> 
> Index: flashrom-emulate_spi_flashchip_command_blacklist/dummyflasher.c
>  
>  
> +	tmp = extract_programmer_param("spi_blacklist");
> +	if (tmp) {

Is there any reason why tmp is declared for whole function instead as variable used only in this block?
Also i would rather name 'tmp' variable 'blacklist_param' or something similar, it isnt reused after as
anything else and would be easier to read code knowing for what it is used for. Also other parameters
have more meaningfull variable names.

> +		tolower_string(tmp);
> +		tmp2 = tmp;
> +		if (!strncmp(tmp2, "0x", 2)) {
> +			tmp2 += 2;
> +		}
> +		i = strlen(tmp2);
> +		if ((i > 512) || (i % 2)) {
> +			msg_perr("Invalid SPI command blacklist length\n");
> +			free(tmp);
> +			return 1;
> +		}
> +		if (strspn(tmp2, "0123456789abcdef") != i) {
> +			msg_perr("Invalid chars in SPI command blacklist\n");
> +			free(tmp);
> +			return 1;
> +		}
> +		spi_blacklist_size = i / 2;
> +		for (i = 0; i < spi_blacklist_size; i++) {
> +			sscanf(tmp2 + i * 2, "%2hhx", &spi_blacklist[i]);
> +		}
> +		msg_pdbg("SPI blacklist is ");
> +		for (i = 0; i < spi_blacklist_size; i++)
> +			msg_pdbg("%02x ", spi_blacklist[i]);
> +		msg_pdbg(", size %i\n", spi_blacklist_size);
> +	}
> +	free(tmp);
> +

> -	/* TODO: Implement command blacklists here. */
> +	for (i = 0; i < spi_blacklist_size; i++) {
> +		if (writearr[0] == spi_blacklist[i]) {
> +			msg_perr("Refusing blacklisted SPI command 0x%02x\n",
> +				 spi_blacklist[i]);
> +			/* FIXME: Do we want a separate return code for
> +			 * command unavailable?
> +			 */

I think, only if non-emulated write may return command unavailable response.

> @@ -444,12 +487,9 @@
>  			msg_perr("CHIP ERASE 0x60 insize invalid!\n");
>  			return 1;
>  		}
> -		offs = writearr[1] << 16 | writearr[2] << 8 | writearr[3];
> -		if (offs & (emu_jedec_ce_60_size - 1))
> -			msg_pdbg("Unaligned CHIP ERASE 0x60\n");

> @@ -462,10 +502,7 @@
>  			msg_perr("CHIP ERASE 0xc7 insize invalid!\n");
>  			return 1;
>  		}
> -		offs = writearr[1] << 16 | writearr[2] << 8 | writearr[3];
> -		if (offs & (emu_jedec_ce_c7_size - 1))
> -			msg_pdbg("Unaligned CHIP ERASE 0xc7\n");

Any word of explain why those two debug messages above are removed?

> Index: flashrom-emulate_spi_flashchip_command_blacklist/flashrom.8

> +If hexstring is 0302, flashrom will blacklist command 0x03 (READ) and
> +command 0x02 (WRITE). The hexstring can have up to 512 characters (256
> +commands) length, and must not start with 0x.

and should contain 0-9a-fA-F characters only? (0x on begin of whole string
is ignored anyway).

I think, along with more options for dummy, manpage could get example how
to pass both, blacklist and something else as parameters for dummy programmer.

sth like -pdummy=SST25VF032B,spi_blacklist=ffeeaa

A bit changed manpage part of patch is in attachment.

-- 
Maciej Pijanka
I don't fear computers, I fear lack of them -- Isaac Asimov
-------------- next part --------------
Index: flashrom.8
===================================================================
--- flashrom.8	(revision 1254)
+++ flashrom.8	(working copy)
@@ -157,11 +157,12 @@
 that MediaWiki output is not compiled in by default.
 .TP
 .B "\-p, \-\-programmer <name>[:parameter[,parameter[,parameter]]]"
-Specify the programmer device. Currently supported are:
+Specify the programmer device, for details please see 'PROGRAMER SPECIFIC INFO' section. 
+Currently supported are:
 .sp
 .BR "* internal" " (default, for in-system flashing in the mainboard)"
 .sp
-.BR "* dummy" " (just prints all operations and accesses)"
+.BR "* dummy" " (prints all operations and accesses, can emulate flash chips)"
 .sp
 .BR "* nic3com" " (for flash ROMs on 3COM network cards)"
 .sp
@@ -320,6 +321,42 @@
 .sp
 Example:
 .B "flashrom \-p dummy:bus=lpc+fwh"
+.sp
+The dummy programmer supports flash chip emulation for automated self-tests
+without hardware access. You have to choose between
+.sp
+* no emulation
+.sp
+* ST M25P10.RES SPI flash chip (RES, page write)
+.sp
+* SST SST25VF040.REMS SPI flash chip (REMS, byte write)
+.sp
+* SST SST25VF032B SPI flash chip (RDID, AAI write)
+.sp
+
+Example usage: flashrom -p dummy:emulate=SST25VF032B
+
+Flash image persistence is available as well.
+Example usage: flashrom -p dummy:image=dummy_simulator.rom
+
+Allow setting the max chunksize for page write with the dummy
+programmer.
+Example usage: flashrom -p dummy:spi_write_256_chunksize=5
+
+optional SPI command blacklisting to the flash chip emulator in the
+dummy programmer.
+
+Usage:
+flashrom -p dummy:spi_blacklist=hexstring
+
+If hexstring is 0302, flashrom will blacklist command 0x03 (READ) and
+command 0x02 (WRITE). The hexstring can have up to 512 characters (256
+commands) length, and must not start with 0x.
+
+Multiple parameters can be specified with 
+.B "flashrom \-p dummy:bus=type[+type[+type]],spi_blacklist=deadbeef" 
+syntax. 
+
 .TP
 .BR "nic3com" , " nicrealtek" , " nicsmc1211" , " nicnatsemi" , " gfxnvidia\
 " , " satasii " and " atahpt " programmers