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

Roman Lebedev lebedev.ri at gmail.com
Sat Dec 20 17:30:09 CET 2014


Hi.

> Third time lucky.
It indeed looks like to be a case here :)

# flashrom --programmer internal -r 1002.bin.patch4 -V
flashrom v0.9.7-r1858 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.patch4 -V
Calibrating delay loop... OS timer resolution is 1 usecs, 534M loops per
second, 10 myus = 11 us, 100 myus = 107 us, 1000 myus = 1024 us, 10000 myus
= 10026 us, 4 myus = 5 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
flashrom at flashrom.org 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 flashrom at flashrom.org 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!
Changed lock bits at 0xb733a002 to 0xf8.
Changed lock bits at 0xb734a002 to 0xf8.
Changed lock bits at 0xb735a002 to 0xf8.
Changed lock bits at 0xb736a002 to 0xf8.
Changed lock bits at 0xb737a002 to 0xf8.
Changed lock bits at 0xb738a002 to 0xf8.
Changed lock bits at 0xb739a002 to 0xf8.
Changed lock bits at 0xb73aa002 to 0xf8.
Reading flash... done.
Restoring PCI config space for 00:1f:0 reg 0x4e


The only two lines that look suspicious are:
    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.
Is that normal?

Thanks for this patches!

Roman.

On Sat, Dec 20, 2014 at 7:07 PM, Stefan Tauner <
stefan.tauner at alumni.tuwien.ac.at> wrote:

> Previously we added the offset of the virtual register in several
> functions,
> which produced segfaults. This patch renames a few parameters and
> reorganizes/fixes various parts of the changelock_regspace2_block()
> function - hence the rather big diff.
>
> Thanks to Roman Lebedev for reporting this issue and testing numerous
> revisions of this patch.
>
> Signed-off-by: Stefan Tauner <stefan.tauner at alumni.tuwien.ac.at>
> ---
>
> $"!%§$&%%$"Q!
> Third time lucky.
> Sorry everybody for the spam.
>
>  chipdrivers.h |  1 -
>  jedec.c       | 93
> ++++++++++++++++++++++++++++++++++-------------------------
>  2 files changed, 53 insertions(+), 41 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..19babf9 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,67 +655,81 @@ 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)
> -{
> -       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 "! "
> +/* Try to change the lock register at address lockreg from cur to new.
> + *
> + * - Try to unlock the lock bit if requested and it is currently set
> (although this is probably futile).
> + * - Try to change the read/write bits if requested.
> + * - Try to set the lockdown bit if requested.
> + * Return an error immediately if any of this fails. */
> +static int changelock_regspace2_block(const struct flashctx *flash,
> chipaddr lockreg, uint8_t cur, uint8_t new)
> +{
> +       /* Only allow changes to known read/write/lockdown bits */
> +       if (((cur ^ new) & ~REG2_MASK) != 0) {
> +               msg_cerr("Invalid lock change from 0x%02x to 0x%02x
> requested at 0x%0*" PRIxPTR "!\n"
>                          "Please report a bug at flashrom at flashrom.org\n",
> -                        new_bits, PRIxPTR_WIDTH, offset);
> +                        cur, 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);
> +
> +       /* Exit early if no change (of read/write/lockdown bits) was
> requested. */
> +       if (((cur ^ new) & REG2_MASK) == 0) {
> +               msg_cdbg2("Lock bits 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);
> +
> +       /* Normally the lockdown bit can not be cleared. Try nevertheless
> if requested. */
> +       if ((cur & REG2_LOCKDOWN) && !(new & REG2_LOCKDOWN)) {
> +               chip_writeb(flash, cur & ~REG2_LOCKDOWN, lockreg);
> +               cur = chip_readb(flash, lockreg);
> +               if ((cur & REG2_LOCKDOWN) == REG2_LOCKDOWN) {
> +                       msg_cwarn("Lockdown can't be removed at 0x%0*"
> PRIxPTR "! New value: 0x%02x.\n",
> +                                 PRIxPTR_WIDTH, lockreg, cur);
>                         return -1;
>                 }
>         }
> -       /* Change read or write lock? */
> -       if ((old ^ new_bits) & REG2_RWLOCK) {
> +
> +       /* Change read and/or write bit */
> +       if ((cur ^ 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);
> +               uint8_t wanted = (cur & ~REG2_RWLOCK) | (new &
> REG2_RWLOCK);
> +               chip_writeb(flash, wanted, lockreg);
> +               cur = chip_readb(flash, lockreg);
> +               if (cur != wanted) {
> +                       msg_cerr("Changing lock bits failed at 0x%0*"
> PRIxPTR "! New value: 0x%02x.\n",
> +                                PRIxPTR_WIDTH, lockreg, cur);
>                         return -1;
>                 }
> +               msg_cdbg("Changed lock bits at 0x%0*" PRIxPTR " to
> 0x%02x.\n",
> +                        PRIxPTR_WIDTH, lockreg, cur);
>         }
> -       /* 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);
> +
> +       /* Eventually, enable lockdown if requested. */
> +       if (!(cur & REG2_LOCKDOWN) && (new & REG2_LOCKDOWN)) {
> +               chip_writeb(flash, new, lockreg);
> +               cur = chip_readb(flash, lockreg);
> +               if (cur != new) {
> +                       msg_cerr("Enabling lockdown FAILED at 0x%0*"
> PRIxPTR "! New value: 0x%02x.\n",
> +                                PRIxPTR_WIDTH, lockreg, cur);
>                         return -1;
>                 }
> +               msg_cdbg("Enabled lockdown at 0x%0*" PRIxPTR ".\n",
> PRIxPTR_WIDTH, lockreg);
>         }
>
>         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 +747,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 +755,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);
>  }
>
> --
> Kind regards, Stefan Tauner
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.flashrom.org/pipermail/flashrom/attachments/20141220/69823ebb/attachment.html>


More information about the flashrom mailing list