Am 14.02.2012 16:39 schrieb Stefan Tauner:
This includes not only the notorious read-only flash descriptors and locked ME regions, but also the more rarely used PRs (Protected Ranges). The user can enforce write support by specifying ich_spi_force=yes in the
Can you rename ich_spi_force to ich_ignore_locks or something similar? The PR stuff is not limited to SPI IIRC.
programmer options, but we don't tell him the exact syntax interactively. He has to read it up in the man page.
non-verbose sample output from my laptop (that contains both protection types): […] Found chipset "Intel QS57". Enabling flash write... WARNING: SPI Configuration Lockdown activated. WARNING: Flash Descriptor region is not fully accessible and flashrom can not deal with this correctly yet. Intel does not provide us the necessary documention to support this. Please send a verbose log to flashrom@flashrom.org if this board is not listed on http://flashrom.org/Supported_hardware#Supported_mainboards yet. Writes have been disabled. You can enforce write support with the ich_spi_force programmer option, but it will most likely harm your hardware! If you force flashrom you will get no support if something breaks. WARNING: Management Engine region is not fully accessible and flashrom can not deal with this correctly yet. WARNING: PR0: 0x007d0000-0x01ffffff is read-only. OK. […]
Ugh. This is the non-verbose version? I had trouble parsing that message flood on the first attempt. May I suggest an alternative wording?
WARNING: Flash Descriptor region is not fully accessible. WARNING: Management Engine region is not fully accessible. WARNING: PR0: 0x007d0000-0x01ffffff is read-only. Please send a verbose log to flashrom@flashrom.org if this board is not listed on http://flashrom.org/Supported_hardware#Supported_mainboards yet. Writes have been disabled. You can force writing with the ich_ignore_locks programmer option, but it will most likely brick your mainboard.
The idea of my wording is to postpone the explanatory message until all "not accessible"/"read-only" messages have been printed.
The complaint about Intel belongs to the man page or to intel_mysteries.txt (if you choose the latter, just mention intel_mysteries.txt at the relevant location in the man page).
For the override case, just print the message above and additionally "You have been warned. We will not support you if write/erase bricks your mainboard. Proceeding anyway because user specified ich_ignore_locks." or something similar.
Signed-off-by: Stefan Tauner stefan.tauner@student.tuwien.ac.at
flashrom.8 | 15 ++++++++++ ichspi.c | 90 +++++++++++++++++++++++++++++++++++++++++++++++------------ 2 files changed, 86 insertions(+), 19 deletions(-)
diff --git a/flashrom.8 b/flashrom.8 index 2f23cb8..0ca4fdb 100644 --- a/flashrom.8 +++ b/flashrom.8 @@ -315,6 +315,21 @@ important opcodes are inaccessible due to lockdown; or if more than one flash chip is attached). The other options (swseq, hwseq) select the respective mode (if possible). .sp +ICH8 and later southbridges may also have locked address ranges of different +kinds if a valid descriptor was written to it. The flash address space is then +partitioned in multiple so called "Flash Regions" containing the host firmware, +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. +.sp If you have an Intel chipset with an ICH6 or later southbridge and if you want to set specific IDSEL values for a non-default flash chip or an embedded controller (EC), you can use the diff --git a/ichspi.c b/ichspi.c index 163ecf1..f21bb35 100644 --- a/ichspi.c +++ b/ichspi.c @@ -1416,12 +1416,36 @@ static int ich_spi_send_multicommand(struct flashctx *flash, return ret; }
+static void ich9_disable_writes(int force, const char * const msg) +{
- /* Don't scare and spam the user even more if write support was already
* disabled */- if (!programmer_may_write)
return;- msg_pinfo("%s", msg);
- msg_pinfo("Please send a verbose log to flashrom@flashrom.org if this "
"board is not listed on\n""http://flashrom.org/Supported_hardware#Supported_mainboards ""yet.\n");- if (!force) {
programmer_may_write = 0;msg_pinfo("Writes have been disabled. You can enforce write ""support with the\nich_spi_force programmer option, ""but it will most likely harm your hardware!\n");- }
- msg_pinfo("If you force flashrom you will get no support if something "
"breaks.\n");- if (force)
msg_pinfo("Continuing anyway because the user forced us to!\n");+}
#define ICH_BMWAG(x) ((x >> 24) & 0xff) #define ICH_BMRAG(x) ((x >> 16) & 0xff) #define ICH_BRWA(x) ((x >> 8) & 0xff) #define ICH_BRRA(x) ((x >> 0) & 0xff)
-static void do_ich9_spi_frap(uint32_t frap, int i) +static void ich9_handle_frap(uint32_t frap, int i, int force) { static const char *const access_names[4] = { "locked", "read-only", "write-only", "read-write" @@ -1447,8 +1471,15 @@ static void do_ich9_spi_frap(uint32_t frap, int i) return; }
- msg_pdbg("0x%08x-0x%08x is %s\n", base, (limit | 0x0fff),
- msg_pdbg("0x%08x-0x%08x is %s.\n", base, (limit | 0x0fff), access_names[rwperms]);
- if (rwperms == 0x3)
return;- msg_pinfo("WARNING: %s region is not fully accessible and flashrom "
"can\nnot deal with this correctly yet.\n", region_names[i]);- ich9_disable_writes(force, "Intel does not provide us the necessary "
"documention to support this.\n");}
/* In contrast to FRAP and the master section of the descriptor the bits @@ -1460,21 +1491,25 @@ static void do_ich9_spi_frap(uint32_t frap, int i) #define ICH_PR_PERMS(pr) (((~((pr) >> PR_RP_OFF) & 1) << 0) | \ ((~((pr) >> PR_WP_OFF) & 1) << 1))
-static void prettyprint_ich9_reg_pr(int i) +static void ich9_handle_pr(int i, int force) {
- static const char *const access_names[4] = {
"locked", "read-only", "write-only", "read-write"
- static const char *const access_names[3] = {
"locked", "read-only", "write-only"
- int rwperms = ICH_PR_PERMS(pr);
- unsigned int rwperms = ICH_PR_PERMS(pr);
- msg_pdbg2("0x%02X: 0x%08x (PR%u", off, pr, i);
- if (rwperms != 0x3)
msg_pdbg2(")\n0x%08x-0x%08x is %s\n", ICH_FREG_BASE(pr),ICH_FREG_LIMIT(pr) | 0x0fff, access_names[rwperms]);- else
msg_pdbg2(", unused)\n");
- if (rwperms >= 0x3) {
Why >= 0x3? AFAICS the ICH_PR_PERMS macro can only have values between 0x0 and 0x3.
msg_pdbg2("0x%02X: 0x%08x (PR%u is unused)\n", off, pr, i);return;- }
- msg_pdbg("0x%02X: 0x%08x ", off, pr);
- msg_pinfo("WARNING: PR%u: 0x%08x-0x%08x is %s.\n", i, ICH_FREG_BASE(pr),
ICH_FREG_LIMIT(pr) | 0x0fff, access_names[rwperms]);- ich9_disable_writes(force, "There is no way to change this from within "
"the system.\n");}
/* Set/Clear the read and write protection enable bits of PR register @i @@ -1537,6 +1572,7 @@ int ich_init_spi(struct pci_dev *dev, uint32_t base, void *rcrb, uint16_t spibar_offset, tmp2; uint32_t tmp; char *arg;
- int ich_spi_force = 0; int desc_valid = 0; struct ich_descriptors desc = {{ 0 }}; enum ich_spi_mode {
@@ -1631,6 +1667,22 @@ int ich_init_spi(struct pci_dev *dev, uint32_t base, void *rcrb, } 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);
@@ -1665,17 +1717,17 @@ int ich_init_spi(struct pci_dev *dev, uint32_t base, void *rcrb, msg_pdbg("BRWA 0x%02x, ", ICH_BRWA(tmp)); msg_pdbg("BRRA 0x%02x\n", ICH_BRRA(tmp));
/* Decode and print FREGx and FRAP registers */
/* Handle FREGx and FRAP registers */ for (i = 0; i < 5; i++)
do_ich9_spi_frap(tmp, i);
ich9_handle_frap(tmp, i, ich_spi_force);
/* try to disable PR locks before printing them */if (!ichspi_lock)for (i = 0; i < 5; i++)
for (i = 0; i < 5; i++) {/* if not locked down try to disable PR locks first */if (!ichspi_lock) ich9_set_pr(i, 0, 0);
for (i = 0; i < 5; i++)prettyprint_ich9_reg_pr(i);
ich9_handle_pr(i, ich_spi_force);}tmp = mmio_readl(ich_spibar + ICH9_REG_SSFS); msg_pdbg("0x90: 0x%02x (SSFS)\n", tmp & 0xff);
Looks good otherwise. Acked-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net
Regards, Carl-Daniel