Change in flashrom[master]: Enable writes with active ME

[Nico Huber (Code Review)]

Nico Huber has uploaded this change for review. ( https://review.coreboot.org/26261

Change subject: Enable writes with active ME
......................................................................

Enable writes with active ME

Replace the `ich_spi_force` logic with more helpful warnings. These can
be hidden later, in case the necessary switches are detected.

Change-Id: I94a5e7074b845c227e43d76d04dd1a71082a1cef
Signed-off-by: Nico Huber 
---
M flashrom.8.tmpl
M ichspi.c
2 files changed, 25 insertions(+), 41 deletions(-)

git pull ssh://review.coreboot.org:29418/flashrom refs/changes/61/26261/1

diff --git a/flashrom.8.tmpl b/flashrom.8.tmpl
index 1600113..019e06a 100644
--- a/flashrom.8.tmpl
+++ b/flashrom.8.tmpl
@@ -488,14 +488,7 @@
 the ME firmware and so on respectively. The flash descriptor can also specify up
 to 5 so called "Protected Regions", which are freely chosen address ranges
 independent from the aforementioned "Flash Regions". All of them can be write
-and/or read protected individually. If flashrom detects such a lock it will
-disable write support unless the user forces it with the
-.sp
-.B "  flashrom -p internal:ich_spi_force=yes"
-.sp
-syntax. If this leads to erase or write accesses to the flash it would most
-probably bring it into an inconsistent and unbootable state and we will not
-provide any support in such a case.
+and/or read protected individually.
 .sp
 If you have an Intel chipset with an ICH2 or later southbridge and if you want
 to set specific IDSEL values for a non-default flash chip or an embedded
diff --git a/ichspi.c b/ichspi.c
index 1989a87..8e5a2aa 100644
--- a/ichspi.c
+++ b/ichspi.c
@@ -221,6 +221,13 @@
 #define ICH7_REG_OPTYPE		0x56	/* 16 Bits */
 #define ICH7_REG_OPMENU		0x58	/* 64 Bits */

+enum ich_access_protection {
+	NO_PROT		= 0,
+	READ_PROT	= 1,
+	WRITE_PROT	= 2,
+	LOCKED		= 3,
+};
+
 /* ICH SPI configuration lock-down. May be set during chipset enabling. */
 static int ichspi_lock = 0;

@@ -1547,8 +1554,7 @@
 #define ICH_BRWA(x)  ((x >>  8) & 0xff)
 #define ICH_BRRA(x)  ((x >>  0) & 0xff)

-/* returns 0 if region is unused or r/w */
-static int ich9_handle_frap(uint32_t frap, int i)
+static enum ich_access_protection ich9_handle_frap(uint32_t frap, int i)
 {
    static const char *const access_names[4] = {
    	"locked", "read-only", "write-only", "read-write"
@@ -1598,7 +1604,7 @@

msg_pwarn("FREG%i: Warning: %s region (0x%08x-0x%08x) is %s.\n", i,
    	  region_name, base, limit, access_names[rwperms]);
-	return 1;
+	return ~rwperms;
 }

/* In contrast to FRAP and the master section of the descriptor the bits
@@ -1610,8 +1616,7 @@
 #define ICH_PR_PERMS(pr)	(((~((pr) >> PR_RP_OFF) & 1) << 0) | \
    			 ((~((pr) >> PR_WP_OFF) & 1) << 1))

-/* returns 0 if range is unused (i.e. r/w) */
-static int ich9_handle_pr(const size_t reg_pr0, int i)
+static enum ich_access_protection ich9_handle_pr(const size_t reg_pr0, int i)
 {
    static const char *const access_names[3] = {
    	"locked", "read-only", "write-only"
@@ -1633,7 +1638,7 @@
    msg_pdbg("0x%02X: 0x%08x ", off, pr);
    msg_pwarn("%sPR%u: Warning: 0x%08x-0x%08x is %s.\n", prefix, i, ICH_FREG_BASE(pr),
    	  ICH_FREG_LIMIT(pr), access_names[rwperms]);
-	return 1;
+	return ~rwperms;
 }

/* Set/Clear the read and write protection enable bits of PR register @i
@@ -1696,7 +1701,6 @@
    uint16_t tmp2;
    uint32_t tmp;
    char *arg;
-	int ich_spi_force = 0;
    int ich_spi_rw_restricted = 0;
    int desc_valid = 0;
    struct ich_descriptors desc = {{ 0 }};
@@ -1805,22 +1809,6 @@
    	}
    	free(arg);

-		arg = extract_programmer_param("ich_spi_force");
-		if (arg && !strcmp(arg, "yes")) {
-			ich_spi_force = 1;
-			msg_pspew("ich_spi_force enabled.\n");
-		} else if (arg && !strlen(arg)) {
-			msg_perr("Missing argument for ich_spi_force.\n");
-			free(arg);
-			return ERROR_FATAL;
-		} else if (arg) {
-			msg_perr("Unknown argument for ich_spi_force: "%s" "
-				 "(not "yes").\n", arg);
-			free(arg);
-			return ERROR_FATAL;
-		}
-		free(arg);
-
    	tmp2 = mmio_readw(ich_spibar + ICH9_REG_HSFS);
    	msg_pdbg("0x04: 0x%04x (HSFS)\n", tmp2);
    	prettyprint_ich9_reg_hsfs(tmp2);
@@ -1876,16 +1864,19 @@
    		ich_spi_rw_restricted |= ich9_handle_pr(reg_pr0, i);
    	}

-		if (ich_spi_rw_restricted) {
-			if (!ich_spi_force)
-				programmer_may_write = 0;
-			msg_pinfo("Writes have been disabled for safety reasons. You can enforce write\n"
-				  "support with the ich_spi_force programmer option, but you will most likely\n"
-				  "harm your hardware! If you force flashrom you will get no support if\n"
-				  "something breaks. On a few mainboards it is possible to enable write\n"
-				  "access by setting a jumper (see its documentation or the board itself).\n");
-			if (ich_spi_force)
-				msg_pinfo("Continuing with write support because the user forced us to!\n");
+		switch (ich_spi_rw_restricted != NO_PROT) {
+		case WRITE_PROT:
+			msg_pwarn("At least some flash regions are write protected. For write operations,\n"
+				  "you should use a flash layout and include only writable regions. See\n"
+				  "manpage for more details.\n");
+			break;
+		case READ_PROT:
+		case LOCKED:
+			msg_pwarn("At least some flash regions are read protected. You have to use a flash\n"
+				  "layout and include only accessible regions. For write operations, you'll\n"
+				  "additionally need the --noverify-all switch. See manpage for more details.\n"
+				  );
+			break;
    	}

tmp = mmio_readl(ich_spibar + swseq_data.reg_ssfsc);