Add protection bits to `struct flash_region` to keep track of the CSME
restrictions for each flash region.
BUG=b:260440773
BRANCH=none
TEST=builds
Change-Id: I0e5b3b4369dc868a8a64338935c5c5249b9a4ada
CoAuthored-by: Edward O'Callaghan <quasisec@google.com>
Signed-off-by: Edward O'Callaghan <quasisec@google.com>
Signed-off-by: Nikolai Artemiev <nartemiev@google.com>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/70437
Reviewed-by: Edward O'Callaghan <quasisec@chromium.org>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
The romentry structure is the container ADT with some
annotated meta-data such as 'included' or 'file' however
the substantive substructure is a 'flash_region'. Therefore
factor this out.
That is to say, the link list node 'romentry' is obscured by the implementation details of its use-case of 'flash_region' that we
clear up here.
BUG=b:260440773
BRANCH=none
TEST=flashrom_tester
Change-Id: I768742b73db901df5b5208fcbcb8a324a06014c2
CoAuthored-by: Nikolai Artemiev <nartemiev@google.com>
Signed-off-by: Nikolai Artemiev <nartemiev@google.com>
Signed-off-by: Edward O'Callaghan <quasisec@google.com>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/69196
Reviewed-by: Edward O'Callaghan <quasisec@chromium.org>
Reviewed-by: Anastasia Klimchuk <aklm@chromium.org>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Move the FCYCLE bit definitions out of the ICH9 definitions and into
their own section because they are used by PCH100 as well.
Rename HSFC_FCYCLE to ICH9_HSFC_FCYCLE because it is specific to ICH9.
BUG=b:253715389,b:253713774
BRANCH=none
TEST=builds
Change-Id: I0996c5331837276049241600e0ffac21a47ec3af
Signed-off-by: Nikolai Artemiev <nartemiev@google.com>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/70458
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Edward O'Callaghan <quasisec@chromium.org>
Reviewed-by: Subrata Banik <subratabanik@google.com>
Reviewed-by: Sam McNally <sammc@google.com>
The global const of `board_matches_size` has value `1` on non-x86
machines. Therefore strictly greater than zero is correct.
Change-Id: Icbe677d3ef164e998daf898ddbea34f96246677f
Signed-off-by: Edward O'Callaghan <quasisec@google.com>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/70554
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Evan Benn <evanbenn@google.com>
Reviewed-by: Anastasia Klimchuk <aklm@chromium.org>
This function works only with spi25 chips
Change-Id: Ie054160b0fdd34bcb128285c6a047e3a3fa8be0c
Signed-off-by: Thomas Heijligen <thomas.heijligen@secunet.com>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/67716
Reviewed-by: Thomas Heijligen <src@posteo.de>
Reviewed-by: Edward O'Callaghan <quasisec@chromium.org>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Split spi_get_opcode_from_erasefn() out into spi.c to add support for
non spi25 flashes next.
Change-Id: Id654e998d0af2d3f5845336bb98b38d724519038
Signed-off-by: Thomas Heijligen <thomas.heijligen@secunet.com>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/67715
Reviewed-by: Thomas Heijligen <src@posteo.de>
Reviewed-by: Edward O'Callaghan <quasisec@chromium.org>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
The non-custom driver programmer delay implementation
'internal_delay()' is unrelated specifically to the
'internal' programmer. The delay implementation is
simply a platform-agnostic host delay implementation.
Therefore, rename to simply default_delay().
Change-Id: I5e04adf16812ceb1480992c92bca25ed80f8897a
Signed-off-by: Edward O'Callaghan <quasisec@google.com>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/68855
Reviewed-by: Alexander Goncharov <chat@joursoir.net>
Reviewed-by: Felix Singer <felixsinger@posteo.net>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Replace `programmer_delay(NULL, [..])` calls with direct
`internal_delay([..])` dispatches explicitly. Custom driver
delays remain hooked as well as core flashrom logic. The
NULL base case of 'programmer_delay()' then becomes a condition
to validate for layering violations or invalid flash contexts.
Change-Id: I1da230804d5e8f47a6e281feb66f381514dc6861
Signed-off-by: Edward O'Callaghan <quasisec@google.com>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/68434
Reviewed-by: Anastasia Klimchuk <aklm@chromium.org>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Handle the special cases of both serprog and ch341a_spi.
Also rewrite programmer_delay() to handle the two base
cases of zero time and no valid flashctx yet before
handling per master branching.
Additionally, modify the custom delay function pointer
signature to allow closure over the flashctx. This allows
driver specific delay implementations to recover programmer
specific opaque data within their delay implementations.
Therefore programmer specific delay functions can avoid
programmer specific globals.
Change-Id: Id059abb58b31a066a408009073912da2b224d40c
Signed-off-by: Edward O'Callaghan <quasisec@google.com>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/67393
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Anastasia Klimchuk <aklm@chromium.org>
Code coverage can be requested with -Dllvm_cov and run with ninja
llvm-cov-tests or llvm-cov-cli.
BUG=b:187647884
BRANCH=None
TEST=meson test; ninja llvm-cov-tests
TEST=ran test_build.sh with coverage enabled
TEST=jenkins ran test_build.sh with coverage disabled
Change-Id: Id6c73bff46e7b88d425956a80def97082b201f56
Signed-off-by: Evan Benn <evanbenn@chromium.org>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/69268
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Anastasia Klimchuk <aklm@chromium.org>
The patch adds one paragraph of information about meson into the
README file. This meant to be the minimum required to unblock
release candidate. README file will have a more substantial
upgrade soon.
Ticket: https://ticket.coreboot.org/issues/354
Change-Id: I2a27d8f2ba42e18be2485ae95bec1b4c874bb4f7
Signed-off-by: Anastasia Klimchuk <aklm@chromium.org>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/70176
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Invert some if conditions to improve the readability of the code.
Instead of running some code if the specific condition applies, error
out early and reduce the indentation levels.
Also, while at it, move the initializers for these for-loops iterators
into their constructs. They are only used by them.
Change-Id: I4021d8802cd041dcca29a226af0798ebd9c5a81b
Signed-off-by: Felix Singer <felixsinger@posteo.net>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/68153
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Edward O'Callaghan <quasisec@chromium.org>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
strdup return values should be checked for NULL to catch the
potential error case of out of memory.
This patch re-writes ternary conditionals so that strdup return
values could be checked for all branches fof execution.
Follow up on
commit 45d50a101e8073191e6d88143990ed91d3bfe815
Ticket: https://ticket.coreboot.org/issues/372
Change-Id: I6c22196be6847a8c9704f1de936604a51b4b8a28
Signed-off-by: Anastasia Klimchuk <aklm@chromium.org>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/70006
Reviewed-by: Edward O'Callaghan <quasisec@chromium.org>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: David Hendricks <david.hendricks@gmail.com>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Code coverage writes data to disk, we need to use real io functions at
this point so that the data is really written.
BUG=b:187647884
BRANCH=None
TEST=llvm-profdata merge -sparse default.profraw -o default.profdata
TEST=llvm-cov show ./flashrom_unit_tests
-instr-profile=default.profdata --format=html --output-dir=.
Change-Id: I21cc1d631e92fa19006b967e85676f108e80b307
Signed-off-by: Evan Benn <evanbenn@chromium.org>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/69267
Reviewed-by: Anastasia Klimchuk <aklm@chromium.org>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Code coverage writes data to disk, we need to use real io functions at
this point so that the data is really written.
BUG=b:187647884
BRANCH=None
TEST=meson test
TEST=ninja coverage
Change-Id: If06053ecd78e886c8f7fc55813f4b5635be78c6b
Signed-off-by: Evan Benn <evanbenn@chromium.org>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/69266
Reviewed-by: Anastasia Klimchuk <aklm@chromium.org>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Implement a check that redirects mock io functions to the real
implementations. Real I/O functions are needed for the coverage tool to
be able to create and write files.
BUG=None
BRANCH=None
TEST=None
Change-Id: I0817fce6ea0f53a4c127794a0d8246504675f805
Signed-off-by: Evan Benn <evanbenn@chromium.org>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/69539
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Anastasia Klimchuk <aklm@chromium.org>
open has a second form with a mode_t argument. When mocking without this
argument a caller trying to O_CREAT would have their mode_t argument
discarded and a random stack variable would be used instead.
BUG=b:187647884
BRANCH=None
TEST=meson test
Change-Id: I8c134e6d36a248d0f51985e389085a9e585fb83d
Signed-off-by: Evan Benn <evanbenn@chromium.org>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/69263
Reviewed-by: Edward O'Callaghan <quasisec@chromium.org>
Reviewed-by: Anastasia Klimchuk <aklm@chromium.org>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
As noted in a comment on
`commit 86fc9cf7ab221bc54ef6f10252e296fc2d7a22d2`, the GD25Q256D
datasheet indicates that the chip does not require a WREN command to
enter 4BA mode.
Testing has confirmed that a WREN command is not required, so change the
flashchip feature flags from FEATURE_4BA_WREN to FEATURE_4BA.
Ticket: https://ticket.coreboot.org/issues/356
BUG=none
BRANCH=none
TEST=read/write/erase/verify GD25Q256D flash with FT2232H programmer
TEST=called spi_enter_exit_4ba(true), dumped registers, checked ADS=1.
Change-Id: I96e48933f33c52c0d10a0d4cb7f7e07c1fceab99
Signed-off-by: Nikolai Artemiev <nartemiev@google.com>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/70342
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Anastasia Klimchuk <aklm@chromium.org>
Reviewed-by: Nico Huber <nico.h@gmx.de>
The config data in the conf object is specific to the man file. Thus,
rename it accordingly and also move the declaration to the place where
it is used.
Change-Id: Icf24b9c47d4c7553f1ec045a9dfd3685b1376445
Signed-off-by: Felix Singer <felixsinger@posteo.net>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/70209
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Thomas Heijligen <src@posteo.de>
The manpage suggests that ft2232_spi chips are only capable of up to
6 MHz SPI clock frequencies, whereas flashrom disables the divide-by-5
prescaler on the 'H' chips allowing up to 30 MHz frequencies. This
detail was already present in the comments of ft2232_spi.c.
Signed-off-by: Nicholas Chin <nic.c3.14@gmail.com>
Change-Id: Id7690e1d4e11a3d0495afbc650f3c67430946468
Reviewed-on: https://review.coreboot.org/c/flashrom/+/70218
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-by: Felix Singer <felixsinger@posteo.net>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Do not just exit in the middle of the process, rather return
a value back up to the caller to allow proper resource cleanup's
to occur.
Change-Id: Ie4186a40071e9a7296d601582ff15ad7df09c70a
Signed-off-by: Edward O'Callaghan <quasisec@google.com>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/69474
Reviewed-by: Felix Singer <felixsinger@posteo.net>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Make the spi25 register read/write functions return SPI_INVALID_OPCODE
if the programmer blocks the read/write opcode for the register.
Likewise, make ichspi read/write register functions return
SPI_INVALID_OPCODE for registers >SR1 as they cannot be accessd.
Make writeprotect ignore SPI_INVALID_OPCODE unless it is trying to
read/write SR1, which should always be supported.
BUG=b:253715389,b:253713774,b:240229722
BRANCH=none
TEST=flashrom --wp-{enable,disable,range,list,status} on dedede
Change-Id: I2145749dcc51f4556550650dab5aa1049f879c45
Signed-off-by: Nikolai Artemiev <nartemiev@google.com>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/69129
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Edward O'Callaghan <quasisec@chromium.org>
Add unit tests for programmer_table, flashchips, and board_matches
structs. The tests are derived from the selfcheck function, checking
that the required fields have been filled in.
BUG=b:140595239
BRANCH=None
TEST=meson test
Change-Id: I41cd014d9bf909296b6c28e3e00548e6883ff41a
Signed-off-by: Evan Benn <evanbenn@chromium.org>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/69620
Reviewed-by: Edward O'Callaghan <quasisec@chromium.org>
Reviewed-by: Anastasia Klimchuk <aklm@chromium.org>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
--include argument was introduced and replaced --image argument in
commit 45d50a101e8073191e6d88143990ed91d3bfe815
This patch cleans up remaining few places where old `--image`
argument was mentioned so that now all the documentation has
`--include`. --image is deprecated.
Both old --image and new --include have the same short version -i
and it remains the same. The code remains the same since the code
handles --include already.
Tested by running
flashrom -h
man ./flashrom.8.tmpl
Ticket: https://ticket.coreboot.org/issues/372
Change-Id: If457e2b8548034868105b515125d7e8b4d5f6134
Signed-off-by: Anastasia Klimchuk <aklm@chromium.org>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/70003
Reviewed-by: Edward O'Callaghan <quasisec@chromium.org>
Reviewed-by: Felix Singer <felixsinger@posteo.net>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Probing an opcode generally shouldn't involve mutating the flashctx
state and currently no probe_opcode functions do that.
Make the flashctx arg const so that call sites don't need to have a
non-const pointer.
BUG=b:253715389,b:253713774
BRANCH=none
TEST=ninja test
Change-Id: I19e98be50d682de2d2715417f8b7b8c62b871617
Signed-off-by: Nikolai Artemiev <nartemiev@google.com>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/70030
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-by: Edward O'Callaghan <quasisec@chromium.org>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Pass by argument the max_rom_decode structure such that the
function is pure and defined upon its parameters.
Note, unfortunately a itermediate step of a '_' suffix is
required for the 'max_rom_decode' parameter as to not alias
the global symbol within the function body.
Change-Id: Ia01f77993deab68e251850008e885828e55b9462
Signed-off-by: Edward O'Callaghan <quasisec@google.com>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/68479
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-by: Felix Singer <felixsinger@posteo.net>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
For compliance and to check if the commit message matches the
development guidelines, add a linter checking for the sign-off-by line
in the commit message. Also, hook it up to the test_build.sh script
which is used for CI builds.
These scripts were copied from the coreboot repository and were adjusted
so that they work for flashrom.
Change-Id: I03f1827803f8492d60a0a44174d5822c2265bfcb
Signed-off-by: Felix Singer <felixsinger@posteo.net>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/70079
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Edward O'Callaghan <quasisec@chromium.org>
Craask and similar DUT's are erroneously probing random second chips.
```
Found chipset "Intel Alder Lake-N".
Enabling flash write... SPI Configuration is locked down.
FREG0: Flash Descriptor region (0x00000000-0x00000fff) is read-write.
FREG1: BIOS region (0x003a0000-0x00ffffff) is read-write.
FREG2: Management Engine region (0x00001000-0x0039ffff) is read-write.
OK.
Found Winbond flash chip "W25Q128.V..M" (16384 kB, Programmer-specific) on host.
Warning: Setting BIOS Control at 0xdc from 0x8b to 0x89 failed.
New value is 0x8b.
Found MoselVitelic flash chip "V29C51000T" (64 kB, Parallel) mapped at physical address 0x00000000ffff0000.
```
This seems to be due to `laptop_ok` becoming a stale global state
after the first operation leading to probing on unrelated buses.
Therefore unconditionally reset the global state upon entry into
the internal driver.
BUG=b:260518132,b:260151917
TEST=Craask reportly no longer finds duplicate chip.
Change-Id: I2c00c351904307eeb1488c5dfaffc91d6468ee25
Signed-off-by: Edward O'Callaghan <quasisec@google.com>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/70026
Reviewed-by: Sam McNally <sammc@google.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Anastasia Klimchuk <aklm@chromium.org>
Flash chips XM25QH256C and XM25QU256C support the 4-byte program
command (0x12) according to their datasheets, but the feature flag is
not enabled in flashchips.c, so enable it to allow this feature to be
used.
TICKET: https://ticket.coreboot.org/issues/371
BUG=b:259493706
TEST=build
Change-Id: I96c80762fcda2af6028c7a53d8c545b0c6565cbd
Signed-off-by: Liam Flaherty <liamflaherty@chromium.org>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/69713
Reviewed-by: Felix Singer <felixsinger@posteo.net>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Anastasia Klimchuk <aklm@chromium.org>
This test upgrades mocks to simulate a read request. Read buffer
is populated with chip manufacture id and chip model id to emulate
successful probing.
TEST=ninja test
Change-Id: I0a2d5591d097435fc69719e1d9bd153433425821
Signed-off-by: Alexander Goncharov <chat@joursoir.net>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/68755
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Anastasia Klimchuk <aklm@chromium.org>
TEST=the following scenarios run tests successfully
1) ch341a_spi is enabled
result: all tests run and pass, including ch341a
2) ch341a_spi is disabled
result: ch341a_spi test is skipped, the rest of tests run and pass
3) libusb isn't presented in the system
result: tests for usb programmers are skipped, the rest of tests run
normally
Change-Id: If28fbe09ad685082152aa3a7e8d5a150169aee9e
Signed-off-by: Alexander Goncharov <chat@joursoir.net>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/67664
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Anastasia Klimchuk <aklm@chromium.org>
This patch adds mocks for several libusb functions are introduced
in one of the previous commits.
Signed-off-by: Alexander Goncharov <chat@joursoir.net>
Change-Id: I5a316687ab39a112d968eeaedb71f7b4b659d8d5
Reviewed-on: https://review.coreboot.org/c/flashrom/+/69873
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Anastasia Klimchuk <aklm@chromium.org>
The patch adds wrappers for libusb functions that are used in the
ch341a programmer and have not yet been introduced.
Change-Id: Ic11efb9fd746cb91911dbe87e1c0028759f5bb0b
Signed-off-by: Alexander Goncharov <chat@joursoir.net>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/69872
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Anastasia Klimchuk <aklm@chromium.org>
Extract the code that converts `struct wp_bits` into a collection of
register values and bit masks out of `write_wp_bits()` into a new
function.
This avoids monadic transformer stacks where unit-testing cannot
penetrate well to give suitable coverage, therefore keep the bit
logic in a separate pure function.
BUG=b:260019525,b:259013033
BRANCH=none
TEST=ninja test
TEST=flashrom --wp-{{dis,en}able,range,list,status} on dedede
Change-Id: I604478ecbb70392c5584bf5d87c76b6f20f882b1
Signed-off-by: Nikolai Artemiev <nartemiev@google.com>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/69846
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Sergii Dmytruk <sergii.dmytruk@3mdeb.com>
Reviewed-by: Edward O'Callaghan <quasisec@chromium.org>
This patch fixes a potential issue where the SPI controller register
HSFC.FDBC (bits 24-29) value gets incorrectly calculated while passing
the `len` as `0` instead of `1`.
As per Intel EDS, `0b` in the FDBC represents 1 byte while `0x3f`
represents 64-bytes to be transferred. The number of bytes
transferred is the value of this field plus 1.
If we would like to transfer 1 byte then we need to set `0b` in
FDBC for operations like read, write, flash id as to account for
the `set byte count` hence, the `len` argument should be `1`.
Additionally, as per EDS, the FDBC field is ignored for any block
erase command.
BUG=b:258280679
TEST=Able to build flashrom and perform below operations on Google,
Rex and Google, Kano/Taeko.
During `--wp-disable` HW seq operation that requires 1 byte data
transfer.
HSFC.FDBC value while passing `len` as `0` = 0x3f (represents 64-byte)
HSFC.FDBC value while passing `len` as `1` = 0x0 (represents 1-byte)
Signed-off-by: Subrata Banik <subratabanik@google.com>
Change-Id: I5b911655649c693e576497520687d7810bbd3c54
Reviewed-on: https://review.coreboot.org/c/flashrom/+/69789
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-by: Edward O'Callaghan <quasisec@chromium.org>
Reviewed-by: Nikolai Artemiev <nartemiev@google.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
This patch fixes a regression introduced with
commit 7ed1337309d3fe74f5af09520970f0f1d417399a (ichspi: Factor out
common hwseq_xfer logic into helpers).
The reason for the regression is ignoring the fact that the Fast SPI
controller MMIO register HSFC (0x06) might not hold the default zero
value before initiating the HW sequencing operation.
Having a `1b` value in the HSFC.FDBC (bits 24-29) field would represent
a byte that needs to be transfered.
While debugging the regression, we have observed that the default value
in the FDBC (prior to initiate any operation) is 0x3f (instead of
zero) which represents 64-byte transfer.
localhost ~ # iotools mmio_read32 0x92d16006
0x3f00
<Fast SPI MMIO BAR: 0x92d16000 and HSFC offset: 0x06>
FDBC offset during `--wp-disable` operation represents higher numbers of
bytes than the actual and eventually results in the error.
Additionally, dropped unused variable (struct hwseq_data *hwseq_data).
BUG=b:258280679
TEST=Able to build flashrom and perform below operations on Google, Rex
and Google, Kano/Taeko.
Without this patch:
HSFC register value inside ich_start_hwseq_xfer() before initiating
the HW seq operations: 0x3f00
HSFC register value inside ich_start_hwseq_xfer() during the HW seq
operations (Read Status): 0x3f11
With this patch:
HSFC register value inside ich_start_hwseq_xfer() before initiating
the HW seq operations: 0x0
HSFC register value inside ich_start_hwseq_xfer() during the HW seq
operations (Read Status): 0x11
Additionally, verified other HW sequencing operations (like read, write,
erase, read status, write status, read ID) working fine without any
error.
Signed-off-by: Subrata Banik <subratabanik@google.com>
Change-Id: I4cc3f24f880d1d621f1f48a6e6b276449fa46f98
Reviewed-on: https://review.coreboot.org/c/flashrom/+/69788
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Edward O'Callaghan <quasisec@chromium.org>
Reviewed-by: Nikolai Artemiev <nartemiev@google.com>
Opaque masters, by design, populate the flashchip structure during
the execution of their probe function. Therefore any opaque master
operation displays a message to the user:
"This flash part has status UNTESTED for operations: WP".
However, for all the other operations (read, write, erase) opaque
masters always mark them as tested. Thus, align WP as marked tested
inline with other opaque chip operations.
BUG=b:258755442
TEST=the following does not display untested message:
1) flashrom -p dummy:size=8388608,emulate=VARIABLE_SIZE
2) flashrom -p internal (on Intel device)
Change-Id: I5ae4cb49eb0abc6ab26cfe2f3359e4e50dd4fd4f
Signed-off-by: Anastasia Klimchuk <aklm@chromium.org>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/69842
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Edward O'Callaghan <quasisec@chromium.org>
Reviewed-by: Sergii Dmytruk <sergii.dmytruk@3mdeb.com>
Reviewed-by: Felix Singer <felixsinger@posteo.net>
partial_lock test was bypassing the WriteProtectState cache of the
software write protect by directly calling env.cmd.wp_range. It was also
unnesesarily disabling software WP. Fix those issues and more clearly
document what the test is doing and expecting.
BUG=b:244663741
BRANCH=None
TEST=flashrom_tester --libflashrom host
Change-Id: Ic3f89ff5d22e74e4e6c94e755b936e58cb27182d
Signed-off-by: Evan Benn <evanbenn@chromium.org>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/69418
Reviewed-by: Peter Marheine <pmarheine@chromium.org>
Reviewed-by: Edward O'Callaghan <quasisec@chromium.org>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
wp_toggle and wp_range had some confusing behaviour where enabling wp
would set a range, but disabling wp would not unset the range
(explicitly). This was a way to workaround the MTD kernel driver
semantics. Now make things very explicit, enabling software write
protect will set the range to the whole chip. Disabling write protect
will set the range to 0,0. This makes all drivers behave the same as
MTD, and documents the exact behaviour explicitly.
BUG=b:244663741
BRANCH=None
TEST=flashrom_tester --libflashrom host # MTD and non-MTD
TEST=flashrom_tester --flashrom_binary # MTD and non-MTD
Change-Id: Ia01d612d988e6580a7c5f0fd448ccc319ce9b181
Signed-off-by: Evan Benn <evanbenn@chromium.org>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/69417
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Edward O'Callaghan <quasisec@chromium.org>
