Nico Huber has uploaded this change for review.

View Change

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 <nico.huber@secunet.com>
---
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);

To view, visit change 26261. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: flashrom
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I94a5e7074b845c227e43d76d04dd1a71082a1cef
Gerrit-Change-Number: 26261
Gerrit-PatchSet: 1
Gerrit-Owner: Nico Huber <nico.h@gmx.de>