On Fri, 02 Sep 2011 20:27:37 +0200 Carl-Daniel Hailfinger <[email protected]> wrote:
> Am 20.08.2011 12:39 schrieb Stefan Tauner: > > Until we are able to unlock regions or at least understand better what > > consequences writing single regions have, disable writing completely > > whenever a non-writeable region is detected. > > > > NB: the registers might not always reflect the real access status. > > http://www.flashrom.org/pipermail/flashrom/2011-April/006309.html > > (this is the sole such report i can remember) > > --- > > Might not be the best location to do this, but currently i don't recognize > > a better > > one. The warnings are repeated for each non-writeable region. We could > > surpress it by checking if programmer_may_write is already 0 before > > printing. > > the output on my thinkpad is: > > […] > > Proceeding anyway because user specified laptop=force_I_want_a_brick > > Found chipset "Intel QS57". Enabling flash write... WARNING: SPI > > Configuration Lockdown activated. > > WARNING: Flash Descriptor region is read-only. Disabling writes. > > WARNING: Management Engine region is locked. Disabling writes. > > OK. > > This chipset supports the following protocols: FWH, SPI. > > Found Winbond flash chip "W25X64" (8192 kB, SPI) at physical address > > 0xff800000. > > […] > > > > verbose output is: > > […] > > 0x54: 0x00000000 (FREG0: Flash Descriptor) > > 0x00000000-0x00000fff is read-only > > WARNING: Flash Descriptor region is read-only. Disabling writes. > > 0x58: 0x07ff0500 (FREG1: BIOS) > > 0x00500000-0x007fffff is read-write > > 0x5C: 0x04ff0003 (FREG2: Management Engine) > > 0x00003000-0x004fffff is locked > > WARNING: Management Engine region is locked. Disabling writes. > > 0x60: 0x00020001 (FREG3: Gigabit Ethernet) > > 0x00001000-0x00002fff is read-write > > 0x64: 0x00000fff (FREG4: Platform Data) > > Platform Data region is unused. > > […] > > > > i think that is ok. > > > > Signed-off-by: Stefan Tauner <[email protected]> > > > > Acked-by: Carl-Daniel Hailfinger <[email protected]> > with two comments, see below. > > > diff --git a/ichspi.c b/ichspi.c > > index 09af2b3..343a0af 100644 > > --- a/ichspi.c > > +++ b/ichspi.c > > @@ -1168,6 +1168,15 @@ static void do_ich9_spi_frap(uint32_t frap, int i) > > > > msg_pdbg("0x%08x-0x%08x is %s\n", base, (limit | 0x0fff), > > access_names[rwperms]); > > + if (rwperms <= 0x1) { > > > > Please make sure that this doesn't trigger for unused regions. if (base > limit) { /* this FREG is disabled */ msg_pdbg("%s region is unused.\n", region_names[i]); return; } is above that hunk... ;) > > + msg_pinfo("WARNING: %s region is %s. Disabling writes.\n", > > + region_names[i], access_names[rwperms]); > > + /* FIXME: This needs some refinement when we know how unlocking > > + * of region works. > > + */ > > + programmer_may_write = 0; > > + } > > + > > } > > > > static const struct spi_programmer spi_programmer_ich7 = { > > > > How do we handle chips where a part of the chip is not part of any > region? What does the hardware enforce in that situation? i am not entirely sure yet if my hypothesis is correct, but i think the hardware prohibits any accesses outside defined regions (i.e. flash cycle error and/or access error log) - most probably only if the flash descriptor is valid of course (and the flash descriptor security override pin is not pulled down)... for some more detail please see [email protected] in the "report for Intel QM67 | Winbond W25Q64" thread. and due to this new information, i am not so sure anymore this patch is a good idea... either we should add a check for PR-based protections and that the regions cover the whole chip too (probably the right thing to do according to my guts), or drop the patch. OTOH improved layout support would make it possible to read/write/verify unprotected parts without a problem. with this patch --force is needed to write them, correct? would be ok with me... as long as it would not completely hinder the user from writing. -- Kind regards/Mit freundlichen Grüßen, Stefan Tauner _______________________________________________ flashrom mailing list [email protected] http://www.flashrom.org/mailman/listinfo/flashrom
