Re: [flashrom] [PATCH] Fix handling of write protection at register space address +2.

[Roman Lebedev]

Hi.

I have tested it, it still produces same correct fw image (same md5), but i
think that the message is still there (unless i horribly miss-apply patch,
which is unlikely)

# flashrom --programmer internal -r 1002.bin.patch2 -V
flashrom v0.9.7-r1857 on Linux 3.16.0-4-686-pae (i686)
flashrom is free software, get the source code at http://www.flashrom.org

flashrom was built with libpci 3.2.1, GCC 4.9.2, little endian
Command line (5 args): flashrom --programmer internal -r 1002.bin.patch2 -V
Calibrating delay loop... OS timer resolution is 1 usecs, 572M loops per
second, 10 myus = 11 us, 100 myus = 147 us, 1000 myus = 1094 us, 10000 myus
= 10740 us, 4 myus = 7 us, OK.
Initializing internal programmer
No coreboot table found.
Using Internal DMI decoder.
DMI string chassis-type: "Desktop"
DMI string system-manufacturer: "ASUSTeK COMPUTER INC."
DMI string system-product-name: "PCH-DR"
DMI string system-version: "1.XX    "
DMI string baseboard-manufacturer: "ASUSTek Computer INC."
DMI string baseboard-product-name: "PCH-DR"
DMI string baseboard-version: "1.XX    "
Found Winbond Super I/O, id 0x82
W836xx enter config mode worked or we were already in config mode. W836xx
leave config mode had no effect.
Active config mode, unknown reg 0x20 ID: 00.
Please send the output of "flashrom -V -p internal" to
[e-mail] with W836xx: your board name: flashrom -V
as the subject to help us finish support for your Super I/O. Thanks.
Found chipset "Intel 6300ESB" with PCI ID 8086:25a1. Enabling flash
write... 0xfff80000/0xffb80000 FWH IDSEL: 0x0
0xfff00000/0xffb00000 FWH IDSEL: 0x0
0xffe80000/0xffa80000 FWH IDSEL: 0x1
0xffe00000/0xffa00000 FWH IDSEL: 0x1
0xffd80000/0xff980000 FWH IDSEL: 0x2
0xffd00000/0xff900000 FWH IDSEL: 0x2
0xffc80000/0xff880000 FWH IDSEL: 0x3
0xffc00000/0xff800000 FWH IDSEL: 0x3
0xff700000/0xff300000 FWH IDSEL: 0x4
0xff600000/0xff200000 FWH IDSEL: 0x5
0xff500000/0xff100000 FWH IDSEL: 0x6
0xff400000/0xff000000 FWH IDSEL: 0x7
0xfff80000/0xffb80000 FWH decode enabled
0xfff00000/0xffb00000 FWH decode enabled
0xffe80000/0xffa80000 FWH decode disabled
0xffe00000/0xffa00000 FWH decode disabled
0xffd80000/0xff980000 FWH decode disabled
0xffd00000/0xff900000 FWH decode disabled
0xffc80000/0xff880000 FWH decode disabled
0xffc00000/0xff800000 FWH decode disabled
0xff700000/0xff300000 FWH decode disabled
0xff600000/0xff200000 FWH decode disabled
0xff500000/0xff100000 FWH decode disabled
0xff400000/0xff000000 FWH decode disabled
Maximum FWH chip size: 0x100000 bytes

BIOS_CNTL = 0x01: BIOS Lock Enable: disabled, BIOS Write Enable: enabled
OK.
The following protocols are supported: FWH.
Probing for Atmel AT49LH002, 256 kB: probe_82802ab: id1 0x08, id2 0x14, id1
is normal flash content, id2 is normal flash content
Probing for Atmel AT49LH00B4, 512 kB: probe_82802ab: id1 0x49, id2 0x4d,
id1 is normal flash content, id2 is normal flash content
Probing for Atmel AT49LH004, 512 kB: probe_82802ab: id1 0x49, id2 0x4d, id1
is normal flash content, id2 is normal flash content
Probing for Intel 82802AB, 512 kB: probe_82802ab: id1 0x49, id2 0x4d, id1
is normal flash content, id2 is normal flash content
Probing for Intel 82802AC, 1024 kB: probe_82802ab: id1 0x49, id2 0x4d, id1
is normal flash content, id2 is normal flash content
Probing for PMC Pm49FL002, 256 kB: probe_jedec_common: id1 0x9d, id2 0x6e
Probing for PMC Pm49FL004, 512 kB: probe_jedec_common: id1 0x9d, id2 0x6e
Found PMC flash chip "Pm49FL004" (512 kB, LPC, FWH) mapped at physical
address 0xfff80000.
Probing for Sharp LHF00L04, 1024 kB: probe_82802ab: id1 0x49, id2 0x4d, id1
is normal flash content, id2 is normal flash content
Probing for SST SST49LF002A/B, 256 kB: probe_jedec_common: id1 0x9d, id2
0x6e
Probing for SST SST49LF003A/B, 384 kB: probe_jedec_common: id1 0x9d, id2
0x6e
Probing for SST SST49LF004A/B, 512 kB: probe_jedec_common: id1 0x9d, id2
0x6e
Probing for SST SST49LF004C, 512 kB: probe_82802ab: id1 0x49, id2 0x4d, id1
is normal flash content, id2 is normal flash content
Probing for SST SST49LF008A, 1024 kB: probe_jedec_common: id1 0x9d, id2 0x6e
Probing for SST SST49LF008C, 1024 kB: probe_82802ab: id1 0x49, id2 0x4d,
id1 is normal flash content, id2 is normal flash content
Probing for SST SST49LF016C, 2048 kB: probe_82802ab: id1 0xff, id2 0xff,
id1 parity violation, id1 is normal flash content, id2 is normal flash
content
Probing for ST M50FLW040A, 512 kB: probe_82802ab: id1 0x49, id2 0x4d, id1
is normal flash content, id2 is normal flash content
Probing for ST M50FLW040B, 512 kB: probe_82802ab: id1 0x49, id2 0x4d, id1
is normal flash content, id2 is normal flash content
Probing for ST M50FLW080A, 1024 kB: probe_82802ab: id1 0x49, id2 0x4d, id1
is normal flash content, id2 is normal flash content
Probing for ST M50FLW080B, 1024 kB: probe_82802ab: id1 0x49, id2 0x4d, id1
is normal flash content, id2 is normal flash content
Probing for ST M50FW002, 256 kB: probe_82802ab: id1 0x08, id2 0x14, id1 is
normal flash content, id2 is normal flash content
Probing for ST M50FW016, 2048 kB: probe_82802ab: id1 0xff, id2 0xff, id1
parity violation, id1 is normal flash content, id2 is normal flash content
Probing for ST M50FW040, 512 kB: probe_82802ab: id1 0x49, id2 0x4d, id1 is
normal flash content, id2 is normal flash content
Probing for ST M50FW080, 1024 kB: probe_82802ab: id1 0x49, id2 0x4d, id1 is
normal flash content, id2 is normal flash content
Probing for Winbond W39V040FA, 512 kB: probe_jedec_common: id1 0x9d, id2
0x6e
Probing for Winbond W39V040FB, 512 kB: probe_jedec_common: id1 0x9d, id2
0x6e
Probing for Winbond W39V040FC, 512 kB: probe_jedec_common: id1 0x9d, id2
0x6e
Probing for Winbond W49V002FA, 256 kB: probe_jedec_common: id1 0x9d, id2
0x6e
Probing for Winbond W39V080FA, 1024 kB: probe_jedec_common: id1 0x9d, id2
0x6e
Probing for Winbond W39V080FA (dual mode), 512 kB: probe_jedec_common: id1
0x9d, id2 0x6e
Found PMC flash chip "Pm49FL004" (512 kB, LPC, FWH).
===
This flash part has status UNTESTED for operations: ERASE WRITE
The test status of this chip may have been updated in the latest development
version of flashrom. If you are running the latest development version,
please email a report to [e-mail] if any of the above
operations
work correctly for you with this flash chip. Please include the flashrom log
file for all operations you tested (see the man page for details), and
mention
which mainboard or programmer you tested in the subject line.
Thanks for your help!
Invalid locking change from 0xf9 to 0xf8 requested at 0xb73cf002!
Please report a bug at [e-mail]
Reading flash... done.
Restoring PCI config space for 00:1f:0 reg 0x4e

Roman.

On Fri, Dec 12, 2014 at 3:25 AM, Stefan Tauner <
[e-mail]> wrote:
> Previously we added the offset of the virtual register in several
> functions cumulatively, which produced segfaults. This patch renames
> a few parameters hence the rather big diff.
>
> Thanks to Roman Lebedev for reporting this issue and testing the patch.
> Signed-off-by: Stefan Tauner
>
> Roman, if you got some time could you please retest this refined version
> of the patch (it is against r1857)? It should behave pretty much the same
> on your setup apart from not showing the message you saw with the last
> version.
>
> chipdrivers.h |  1 -
>  jedec.c       | 65
> +++++++++++++++++++++++++++--------------------------------
>  2 files changed, 30 insertions(+), 36 deletions(-)
>
> diff --git a/chipdrivers.h b/chipdrivers.h
> index 8529c74..cac94f3 100644
> --- a/chipdrivers.h
> +++ b/chipdrivers.h
> @@ -150,7 +150,6 @@ int unlock_regspace2_uniform_32k(struct flashctx
> *flash);
>  int unlock_regspace2_uniform_64k(struct flashctx *flash);
>  int unlock_regspace2_block_eraser_0(struct flashctx *flash);
>  int unlock_regspace2_block_eraser_1(struct flashctx *flash);
> -int unlock_regspace2_block(const struct flashctx *flash, chipaddr off);
>  int printlock_regspace2_uniform_64k(struct flashctx *flash);
>  int printlock_regspace2_block_eraser_0(struct flashctx *flash);
>  int printlock_regspace2_block_eraser_1(struct flashctx *flash);
> diff --git a/jedec.c b/jedec.c
> index 1345b89..6de6745 100644
> --- a/jedec.c
> +++ b/jedec.c
> @@ -589,11 +589,10 @@ static int regspace2_walk_unlockblocks(const struct
> flashctx *flash, const struc
>  #define REG2_LOCKDOWN (1 << 1)
>  #define REG2_MASK (REG2_RWLOCK | REG2_LOCKDOWN)
>
> -static int printlock_regspace2_block(const struct flashctx *flash,
> chipaddr offset)
> +static int printlock_regspace2_block(const struct flashctx *flash,
> chipaddr lockreg)
>  {
>   chipaddr wrprotect = flash->virtual_registers + offset + 2;
>   uint8_t state = chip_readb(flash, wrprotect);
>   msg_cdbg("Lock status of block at 0x%0*" PRIxPTR " is ",
>
> PRIxPTR_WIDTH, offset);
>   uint8_t state = chip_readb(flash, lockreg);
>   msg_cdbg("Lock status of block at 0x%0*" PRIxPTR " is ",
>
> PRIxPTR_WIDTH, lockreg);
>         switch (state & REG2_MASK) {
>         case 0:
>                 msg_cdbg("Full Access.\n");
> @@ -656,47 +655,44 @@ int printlock_regspace2_block_eraser_1(struct
> flashctx *flash)
>         return regspace2_walk_unlockblocks(flash, unlockblocks,
> &printlock_regspace2_block);
>  }
>
> -static int changelock_regspace2_block(const struct flashctx *flash,
> chipaddr offset, uint8_t new_bits)
> +static int changelock_regspace2_block(const struct flashctx *flash,
> chipaddr lockreg, uint8_t old, uint8_t new)
>  {
>   chipaddr wrprotect = flash->virtual_registers + offset + 2;
>   uint8_t old;
>   if (new_bits & ~REG2_MASK) {
>           msg_cerr("Invalid locking change 0x%02x requested at
>
> 0x%0*" PRIxPTR "! "
>   if (((old ^ new) & REG2_MASK) != 0) {
>           msg_cerr("Invalid locking change from 0x%02x to 0x%02x
>
> requested at 0x%0*" PRIxPTR "!\n"
>                          "Please report a bug at [e-mail]\n",
>                    new_bits, PRIxPTR_WIDTH, offset);
>                    old, new, PRIxPTR_WIDTH, lockreg);
>           return -1;
>   }
>   old = chip_readb(flash, wrprotect);
>   /* Early exist if no change (of read/write/lockdown) was
>
> requested. */
>   if (((old ^ new_bits) & REG2_MASK) == 0) {
>           msg_cdbg2("Locking status at 0x%0*" PRIxPTR " not
>
> changed\n", PRIxPTR_WIDTH, offset);
>   if (((old ^ new) & REG2_MASK) == 0) {
>           msg_cdbg2("Locking status at 0x%0*" PRIxPTR " not
>
> changed\n", PRIxPTR_WIDTH, lockreg);
>                 return 0;
>         }
>         /* Normally lockdowns can not be cleared. Try nevertheless if
> requested. */
>   if ((old & REG2_LOCKDOWN) && !(new_bits & REG2_LOCKDOWN)) {
>           chip_writeb(flash, old & ~REG2_LOCKDOWN, wrprotect);
>           if (chip_readb(flash, wrprotect) != (old &
>
> ~REG2_LOCKDOWN)) {
>                   msg_cerr("Lockdown can't be removed at 0x%0*"
>
> PRIxPTR "!\n", PRIxPTR_WIDTH, offset);
>   if ((old & REG2_LOCKDOWN) && !(new & REG2_LOCKDOWN)) {
>           chip_writeb(flash, old & ~REG2_LOCKDOWN, lockreg);
>           if (chip_readb(flash, lockreg) != (old & ~REG2_LOCKDOWN)) {
>                   msg_cerr("Lockdown can't be removed at 0x%0*"
>
> PRIxPTR "!\n", PRIxPTR_WIDTH, lockreg);
>                         return -1;
>                 }
>         }
>         /* Change read or write lock? */
>   if ((old ^ new_bits) & REG2_RWLOCK) {
>   if ((old ^ new) & REG2_RWLOCK) {
>           /* Do not lockdown yet. */
>           msg_cdbg("Changing locking status at 0x%0*" PRIxPTR " to
>
> 0x%02x\n", PRIxPTR_WIDTH, offset, new_bits & REG2_RWLOCK);
>           chip_writeb(flash, new_bits & REG2_RWLOCK, wrprotect);
>           if (chip_readb(flash, wrprotect) != (new_bits &
>
> REG2_RWLOCK)) {
>                   msg_cerr("Locking status change FAILED at 0x%0*"
>
> PRIxPTR "!\n", PRIxPTR_WIDTH, offset);
>           msg_cdbg("Changing locking status at 0x%0*" PRIxPTR " to
>
> 0x%02x\n",
>                    PRIxPTR_WIDTH, lockreg, new & REG2_RWLOCK);
>           chip_writeb(flash, new & REG2_RWLOCK, lockreg);
>           if (chip_readb(flash, lockreg) != (new & REG2_RWLOCK)) {
>                   msg_cerr("Locking status change FAILED at 0x%0*"
>
> PRIxPTR "!\n", PRIxPTR_WIDTH, lockreg);
>                         return -1;
>                 }
>         }
>         /* Enable lockdown if requested. */
>   if (!(old & REG2_LOCKDOWN) && (new_bits & REG2_LOCKDOWN)) {
>           msg_cdbg("Enabling lockdown at 0x%0*" PRIxPTR "\n",
>
> PRIxPTR_WIDTH, offset);
>           chip_writeb(flash, new_bits, wrprotect);
>           if (chip_readb(flash, wrprotect) != new_bits) {
>                   msg_cerr("Enabling lockdown FAILED at 0x%0*"
>
> PRIxPTR "!\n", PRIxPTR_WIDTH, offset);
>   if (!(old & REG2_LOCKDOWN) && (new & REG2_LOCKDOWN)) {
>           msg_cdbg("Enabling lockdown at 0x%0*" PRIxPTR "\n",
>
> PRIxPTR_WIDTH, lockreg);
>           chip_writeb(flash, new, lockreg);
>           if (chip_readb(flash, lockreg) != new) {
>                   msg_cerr("Enabling lockdown FAILED at 0x%0*"
>
> PRIxPTR "!\n", PRIxPTR_WIDTH, lockreg);
>                         return -1;
>                 }
>         }
> @@ -704,19 +700,18 @@ static int changelock_regspace2_block(const struct
> flashctx *flash, chipaddr off
>         return 0;
>  }
>
> -int unlock_regspace2_block(const struct flashctx *flash, chipaddr off)
> +static int unlock_regspace2_block_generic(const struct flashctx *flash,
> chipaddr lockreg)
>  {
>   chipaddr wrprotect = flash->virtual_registers + off + 2;
>   uint8_t old = chip_readb(flash, wrprotect);
>   uint8_t old = chip_readb(flash, lockreg);
>   /* We don't care for the lockdown bit as long as the RW locks are
>
> 0 after we're done */
>   return changelock_regspace2_block(flash, off, old & ~REG2_RWLOCK);
>   return changelock_regspace2_block(flash, lockreg, old, old &
>
> ~REG2_RWLOCK);
>  }
>
> static int unlock_regspace2_uniform(struct flashctx *flash, unsigned long
> block_size)
>  {
>         const unsigned int elems = flash->chip->total_size * 1024 /
> block_size;
>         struct unlockblock blocks[2] = {{.size = block_size, .count =
> elems}};
>   return regspace2_walk_unlockblocks(flash, blocks,
>
> &unlock_regspace2_block);
>   return regspace2_walk_unlockblocks(flash, blocks,
>
> &unlock_regspace2_block_generic);
>  }
>
> int unlock_regspace2_uniform_64k(struct flashctx *flash)
> @@ -734,7 +729,7 @@ int unlock_regspace2_block_eraser_0(struct flashctx
> *flash)
>         // FIXME: this depends on the eraseblocks not to be filled up
> completely (i.e. to be null-terminated).
>         const struct unlockblock *unlockblocks =
>                 (const struct unlockblock
> *)flash->chip->block_erasers[0].eraseblocks;
>   return regspace2_walk_unlockblocks(flash, unlockblocks,
>
> &unlock_regspace2_block);
>   return regspace2_walk_unlockblocks(flash, unlockblocks,
>
> &unlock_regspace2_block_generic);
>  }
>
> int unlock_regspace2_block_eraser_1(struct flashctx *flash)
> @@ -742,6 +737,6 @@ int unlock_regspace2_block_eraser_1(struct flashctx
> *flash)
>         // FIXME: this depends on the eraseblocks not to be filled up
> completely (i.e. to be null-terminated).
>         const struct unlockblock *unlockblocks =
>                 (const struct unlockblock
> *)flash->chip->block_erasers[1].eraseblocks;
>   return regspace2_walk_unlockblocks(flash, unlockblocks,
>
> &unlock_regspace2_block);
>   return regspace2_walk_unlockblocks(flash, unlockblocks,
>
> &unlock_regspace2_block_generic);
>  }