Re: [flashrom] [PATCH] ichspi.c: warn user when a protected region is detected
On Sun, 27 Nov 2011 11:48:51 -0800 David Hendricks <david.hendricks@gmail.com> wrote:
On Sat, Nov 26, 2011 at 3:35 PM, Stefan Tauner < stefan.tauner@student.tuwien.ac.at> wrote:
This includes the notorious read-only flash descriptors and locked ME regions. --- non-verbose sample output from my laptop: […] 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.
To be fair, I think Intel documents it fine.
That depends on what 'it' is. The limitations and the influence of FDOPSS on that limitation are well defined in public documentation. But the unlocking process is not documented at all publicly. We know from different leaked documents and also from the fact that vendor tools exist, that unlocking can be done by software only and without touching the FDOPSS pin by sending the "HMRFPO Enable" command via HECI/MEI to the ME. The details are documented in the BIOS writer guide(s) (which are "restricted secret" level(?))
I think what we've got to do is checking the flash descriptor override pin strap status (FDOPSS). If it is cleared then we can ignore the descriptor, otherwise if it is set then we need to avoid locked regions.
I would not call it 'ignoring'. We should be aware, that the limitation do not apply (we do print a message to the user already in that case), but we could and should use the regions where it makes sense (e.g. automatic creation of layout (file)s.
It's really just a pain in the ass and, as you pointed out, may leave the BIOS/ME firmware blobs in an inconsistent or incompatible state. So the onus is on the user to ensure a safe upgrade path if only part of the ROM can be updated. It's probably worth displaying a warning and requiring "--force" or something in that scenario.
As a first step yes. IIRC i have sent a patch that does that when active PR protections are found(?), but i think it is not in/reviewed yet. I agree, we should set write_allowed = 0 (or whatever it was) and rephrase the warning to include that. -- Kind regards/Mit freundlichen Grüßen, Stefan Tauner
Am 27.11.2011 22:27 schrieb Stefan Tauner:
On Sun, 27 Nov 2011 11:48:51 -0800 David Hendricks <david.hendricks@gmail.com> wrote:
On Sat, Nov 26, 2011 at 3:35 PM, Stefan Tauner < stefan.tauner@student.tuwien.ac.at> wrote:
This includes the notorious read-only flash descriptors and locked ME regions. --- non-verbose sample output from my laptop: […] 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.
To be fair, I think Intel documents it fine. That depends on what 'it' is. The limitations and the influence of FDOPSS on that limitation are well defined in public documentation. But the unlocking process is not documented at all publicly. We know from different leaked documents and also from the fact that vendor tools exist, that unlocking can be done by software only and without touching the FDOPSS pin by sending the "HMRFPO Enable" command via HECI/MEI to the ME. The details are documented in the BIOS writer guide(s) (which are "restricted secret" level(?))
I think what we've got to do is checking the flash descriptor override pin strap status (FDOPSS). If it is cleared then we can ignore the descriptor, otherwise if it is set then we need to avoid locked regions. I would not call it 'ignoring'. We should be aware, that the limitation do not apply (we do print a message to the user already in that case), but we could and should use the regions where it makes sense (e.g. automatic creation of layout (file)s.
It's really just a pain in the ass and, as you pointed out, may leave the BIOS/ME firmware blobs in an inconsistent or incompatible state. So the onus is on the user to ensure a safe upgrade path if only part of the ROM can be updated. It's probably worth displaying a warning and requiring "--force" or something in that scenario. As a first step yes. IIRC i have sent a patch that does that when active PR protections are found(?), but i think it is not in/reviewed yet. I agree, we should set write_allowed = 0 (or whatever it was) and rephrase the warning to include that.
Do you want to keep the message as-is or do you want to make some changes? I don't have a strong preference either way. And do you want to set programmer_may_write=0 here? Acked-by: Carl-Daniel Hailfinger <c-d.hailfinger.devel.2006@gmx.net> Regards, Carl-Daniel -- http://www.hailfinger.org/
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 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. […] 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" }; uint8_t off = ICH9_REG_PR0 + (i * 4); uint32_t pr = mmio_readl(ich_spibar + off); - 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) { + 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); -- 1.7.1
participants (2)
-
Carl-Daniel Hailfinger -
Stefan Tauner