Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Initial status register, access protection and OTP infrastructure #60

Open
wants to merge 12 commits into
base: master
Choose a base branch
from

Conversation

sch-m
Copy link
Contributor

@sch-m sch-m commented Oct 4, 2018

This PR should bring the work of Hatim Kanchwala upstream.

Here are the original descriptions of the patch-sets:

Initial status register and access protection infrastructure:

Following patches add new infrastructure for status register(s) and locking/unlocking of access protection. Detailed points for explaining each patch are included within them.

Some general thoughts -

  • The code has a lot TODOs and FIXMEs (most of which I have addressed to myself). They are short-term targets and represent the work-in-progress nature of the patches. Future revisions will resolve them.
  • There are a lot of comments. Eventually much of that will be relocated to documentation and wiki, so comments in future revisions will shrink.
  • Some CLI code is borrowed from the ChromiumOS fork (https://chromium.googlesource.com/chromiumos/third_party/flashrom/). IMO, the exact words to use for the options is subjective.
  • The names for struct status_register and struct wp defines (in statusreg_layouts.c and writeprotect_layouts.c respectively) are not very creative and tend to be long. I couldn't come up with a generic scheme that captures most of the information regarding it. You are welcome to offer creative suggestions ;)

Further work -

  • Read and write "configuration" registers (a lot of Spansion chips have those)
  • Add tested/untested status for new infrastructure similar to what we have for chips (like TEST_PREW, etc.)
    • write protection modes
    • protection ranges
  • Access protection for non-SPI chips (pointed to by Stefan over IRC)
  • Reuse layouts for locking/unlocking (pointed to by Stafan and David over IRC)
  • Handling WPS bit (a couple of GigaDevice chips have this)
  • Exotic chips (mostly Atmel, now Adesto, chips fall under this category)

This is what I particularly seek -

  • Feedback on code
  • Feedback on CLI
  • Any other ideas that you'd like to add tu further work
  • Please add support for a few chips that you might have, and if possible also test them - would like to know how cumbersome it is

Thank you for your time. I am looking forward to your feedback and having a discussion. I hope this contribution (and many more in the future) adds value to flashrom and its community.

Initial OTP/Security Register infrastructure:

This set of patches should be applied on top of the previous patches (https://www.flashrom.org/pipermail/flashrom/2016-July/014717.html).

Continuing with the major points from the previous mail (linked above), this patch should bring support for new infrastructure to quite a few more chips. OTP models for Eon, GigaDevice and Winbond are supported now. AMIC, Macronix and Spansion OTP models are scheduled for the next revision. I admit that the CLI may not be worded in the most optimum way, but that was a conscious decision so that it could be sorted out while review.

- Read and write multiple status registers
  - Read is straightforward and most chips share same opcodes for RDSR1, RDS2 and RDSR3
  - For chips with 2 status registers, WRSR takes either 1 or 2 bytes. When only 1 byte is supplied (which was the preious behaviour), the 2nd status register is cleared. (Our code automatically takes care of this.)
  - For chips with 3 status registers, each register is separately written to and we have many chips sharing opcodes for WRSR1, WRSR2 and WRSR3
- Get, set or prettyprint write protection mode for status register(s). Functionality exposed through struct status_register member.
  - This is controlled by SRP/SRWD or SRP0/SRP1 bit(s). Chips with SRP0 and SRP1 will most likely have at least 2 status registers.
  - For chips with SRP/SRWD bit, we can get/set SOFTWARE (status register unlocked) or HARDWARE (status register locked/unlocked subject to WP#) protection modes.
  - For chips with SRP0 and SRP1, we can additionally get/set POWER_CYCLE (status registers locked until next power down-up cycle) or PERMANENT modes.
  - We can also automatically detect how WP# affects the HARDWARE write protection mode.
- struct flashchip contains pointer to a struct status_register (for allowing flexibility of reuse), which in turn has members to represent layout of status register(s) and, pointers to functions to read, write, print, set_wp_mode, get_wp_mode, print_wp_mode. Function pointers allow flexibility to assign chip specific routines for exotic cases.
- Prettyprinting of different status register bits is unified. Newer bits can be defined in enum status_register_bit and description written in statreg_bit_desc[][]. Functionality exposed through struct status_register member.

Martin Schiller:
----------------
- Do not use 'for' loop initial declarations to avoid
  dependency to C99/C11 mode.

Signed-off-by: Hatim Kanchwala <hatim@hatimak.me>
Signed-off-by: Martin Schiller <ms@dev.tdt.de>
- struct flashchip contains pointer to a struct wp (for allowing flexibility of reuse), which in turn has members to represent protection ranges and, pointers to functions to fetch bp_bitmask, set protection range, disable any protection, and print protection range table. Function pointers allow flexibility to assign chip specific routines for exotic cases.
- Print table of valid ranges for write protection
  - Automatically fetches bit names from status_register->layout to make list more meaningful
- Disabling of block protection is unified (we can decommission the various spi_disable_blockprotect() functions from spi25_statusreg.c)
  - Write protection mode(s) of status register(s) are automatically queried and disabled (courtesy of struct status_register)
  - BP bitmasks are fetched from wp->bitmask
- For around 50% of chips supported by flashrom (as of writing this), ranges are automatically generated based. This is especially true of GigaDevice and Winbond chips. For many other chips the generated range can be used as boilerplate to start with.
  - The presence of a CMP bit is automatically handled for printing as well as setting range (as long as a standard is followed - please refer comment under compute_cmp_ranges label in sec_block_range_pattern() function in writeprotect.c)

Martin Schiller:
----------------
- Do not use 'for' loop initial declarations to avoid
  dependency to C99/C11 mode.

Signed-off-by: Hatim Kanchwala <hatim@hatimak.me>
Signed-off-by: Martin Schiller <ms@dev.tdt.de>
- struct status_register defines added for the following chips (66 in total) -
  - AMIC(4) : A25L080, A25LQ16, A25LQ32A, A25L032
  - Macronix(15) : MX25L6408E, MX25L6406E, MX25L1605D, MX25L3205D, MX25L6405D, MX25L1608D, MX25L3208D, MX25L6408D, MX25L6436E, MX25L6445E, MX25L6465E, MX25L12865E, MX25L12845E, MX25L12835F, MX25L1673E
  - GigaDevice(41) : GD25LQ16, GD25LQ40, GD25LQ80B, GD25LQ40B, GD25LQ64C, GD25LQ80, GD25LQ128C, GD25LQ32C, GD25LQ16, GD25LQ40, GD25LQ80B, GD25LQ40B, GD25LQ64C, GD25LQ80, GD25LQ128C, GD25LQ32C, GD25Q16B, GD25Q32B, GD25Q64B, GD25Q10, GD25Q16, GD25Q20, GD25Q40, GD25Q80, GD25VQ16C, GD25VQ80C, GD25Q16C, GD25Q40C, GD25VQ21B, GD25VQ41B, GD25Q21B, GD25Q41B, GD25Q80B, GD25Q128, GD25LQ05B, GD25LQ10B, GD25LQ20B, GD25Q32C, GD25Q64C, GD25Q127C, GD25Q128C
  - Winbond(6) : W25Q80, W25Q16, W25Q32, W25Q40BL, W25Q64FV, W25Q128FW
- 16 of the above support new infrastructure (in flashchips.c) (WIP)
- 19 unique struct definitions were required to represent all of the above chips.
- Note that quite a few chips don't have support in flashrom (yet).

Signed-off-by: Hatim Kanchwala <hatim@hatimak.me>
Signed-off-by: Martin Schiller <ms@dev.tdt.de>
- struct wp defines added for the following chips (21 in total) -
  - AMIC(4) : A25LQ032, A25LQ32A, A25L080, A25LQ16
  - Macronix(12) : MX25L1605D, MX25L1608D, MX25L1673E, MX25L6406E, MX25L6408E, MX25L6405D, MX25L3205D, MX25L3208D, MX25L6436E, MX25L6445E, MX25L6465E, MX25L6473E
  - GigaDevice(5) : GD25LQ40, GD25LQ80, GD25LQ16, GD25Q16, GD25Q16B
- All of the above support new infrastructure (in flashchips.c) (WIP)
- 6 unique struct definitions were required to represent all of the above chips

Signed-off-by: Hatim Kanchwala <hatim@hatimak.me>
Signed-off-by: Martin Schiller <ms@dev.tdt.de>
- New infrastructure is used to read status register (for chips that have support for struct status_register) throughout spi25.c.
- New infrastructure is used to prettyprint status register and write protection mode of status register (for chips that have support for struct status_register) in flashrom.c.
- New disable from access protection infrastructure is used (for chips that have support for struct wp) in flashrom.c

Martin Schiller:
----------------
- Do not use 'for' loop initial declarations to avoid
  dependency to C99/C11 mode.

Signed-off-by: Hatim Kanchwala <hatim@hatimak.me>
Signed-off-by: Martin Schiller <ms@dev.tdt.de>
- Following new CLI options added -
  - print-status-reg : print detailed contents of status register(s)
  - print-wp-status : print write protection mode of status register(s)
  - wp-list : print list of write protection ranges
  - wp-enable : enable write protection of status register(s) to optionally supplied MODE argument
  - wp-disable : disable any write protection of status register(s)
  - wp-set-range : set write protection range to supplied range
- Updated man page with new CLI options

Martin Schiller:
----------------
- Do not use 'for' loop initial declarations to avoid
  dependency to C99/C11 mode.
- Some small compatibility changes for long_opts

Signed-off-by: Hatim Kanchwala <hatim@hatimak.me>
Signed-off-by: Martin Schiller <ms@dev.tdt.de>
…ture to more existing chips

- Infrastructure support in flashchips.c added for (28 in total) -
  - Eon(13) : EN25QH128, EN25Q128, EN25QH64, EN25Q64, EN25QH32, EN25QH16, EN25Q16, EN25Q32(A/B), EN25Q80(A), EN25Q40
  - GigaDevice(13) : GD25Q80(B), GD25Q32(B), GD25Q64(B), GD25Q128B, GD25Q128C, GD25VQ21B, GD25VQ40C, GD25VQ41B, GD25VQ80C, GD25VQ16C
  - Winbond(2) : W25Q40BL, W25Q64FV

Signed-off-by: Hatim Kanchwala <hatim@hatimak.me>
Signed-off-by: Martin Schiller <ms@dev.tdt.de>
- Read, write or erase OTP memory
  - Two distinct models (not exhaustive) -
    - Security sector (Eon) - Separate memory array accessible as a normal sector while in OTP mode, WRSR locks it permanently
    - Security Registers (GigaDevice and Winbond) - Separate memory location with dedicated opcodes for read, program and erase; OTP status controlled by Lock Bits (LB1, LB2, ...) in status register(s)
- struct region represents a quantum of OTP memory - start address, size and modifier bit in status register (generic design applies to all aforementioned models)
- struct flashchip contains pointer to struct otp, which has members to represent OTP regions and function pointers to fetch and print status, read, write, erase and lock OTP regions

Martin Schiller:
----------------
- read/write the security register in chunks, because some spi
  controllers (e.g. AMD SB800 in my case) are not able to
  read/write the whole register at once.
- Merged some fixes from another commit of Hatim:
  - eon_(read/write)_generic(): start addr fix

Signed-off-by: Hatim Kanchwala <hatim@hatimak.me>
Signed-off-by: Martin Schiller <ms@dev.tdt.de>
… chips

Infrastructure support in flashchips.c added for (31 in total) -
  - Eon(13) : EN25Q40, EN25Q80(A), EN25Q16, EN25Q32(A/B), EN25Q64, EN25Q128, EN25QH16, EN25QH32, EN25QH64, EN25QH128
  - GigaDevice(16) : GD25LQ40, GD25LQ80, GD25LQ16, GD25Q80B, GD25Q16B, GD25Q32(B), GD25Q64(B), GD25Q128B, GD25Q128C, GD25VQ21B, GD25VQ40C, GD25VQ41B, GD25VQ80C, GD25VQ16C
  - Winbond(2) : W25Q40BL, W25Q64FV

Signed-off-by: Hatim Kanchwala <hatim@hatimak.me>
Signed-off-by: Martin Schiller <ms@dev.tdt.de>
- Following CLI options added -
  - print-otp-status : print OTP memory regions and lock status
  - read-otp : read OTP memory (optionally specifiy region) and save to <file>
  - write-otp : write <file> to OTP memory (optionally specifiy region)
  - erase-otp : erase OTP memory (optionally specifiy region)
  - lock-otp : lock OTP memory (optionally specifiy region)
- Updated man page with new CLI options
- Updated message for chips with FEATURE_OTP, but which do not yet support new infrastructure

Martin Schiller:
----------------
- Some small compatibility changes for long_opts
- Merged some fixes from another commit of Hatim:
  - Fix option parsing for erase_otp_opt and lock_otp_opt
  - Fix copy'N'paste error for erase_otp_opt and lock_otp_opt

Signed-off-by: Hatim Kanchwala <hatim@hatimak.me>
Signed-off-by: Martin Schiller <ms@dev.tdt.de>
GD25Q128C -
  - 3 status registers each with separate read and write opcodes for corresponding SRs
  - 3 security registers each of 512 bytes

EN25QH128 -
  - Single status register
  - OTP sector of 512 bytes

W25Q40.V (W25Q40BL) -
  - 2 status registers each with separate read but with single write opcode
  - 3 security registers each of 256 bytes

Signed-off-by: Hatim Kanchwala <hatim@hatimak.me>
Signed-off-by: Martin Schiller <ms@dev.tdt.de>
Uses new status register, access protection and OTP infrastructure (WIP). Chip definition for "W25Q128.V" needed to be separated into 2 owing to different status register configurations of W25Q128BV and W25Q128FV.

Signed-off-by: Hatim Kanchwala <hatim@hatimak.me>
Signed-off-by: Martin Schiller <ms@dev.tdt.de>
@milahu
Copy link

milahu commented Oct 24, 2020

make fails with

/usr/bin/ld: developerbox_spi.o:(.bss+0x0): multiple definition of `usb_ctx'; dediprog.o:(.bss+0x8): first defined here

$ grep 'usb_ctx;' *.c
dediprog.c:struct libusb_context *usb_ctx;
developerbox_spi.c:struct libusb_context *usb_ctx;

workaround:

sed -i 's/usb_ctx/dediprog_usb_ctx/' dediprog.c
make
# success

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants