The main purpose is to run one operation after the other to
check that data is carried in flash context correctly.
As the most common chains, the first tests perform:
probe+read
probe+write
probe+erase
Change-Id: I9b09e04c7dbee7e7658118d66aacb640885f4d23
Signed-off-by: Anastasia Klimchuk <aklm@flashrom.org>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/88257
Reviewed-by: Stefan Reinauer <stefan.reinauer@coreboot.org>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Previously probe_flash had the same return code for the case when
no chips were matched, and when some other error happened during
probing. However these are two different scenarios and it is useful
for the caller to distinguish between them.
In fact, the caller (libflashrom it is) wanted to distinguish
between "no chips found" and "some other probing error" from the
very beginning. libflashrom probe API documented returning special
error code for "other error".
However it was not possible to know when "other error" happened
because probe_flash never returned that back, it could only say
"no matched chips found".
This patch introduces -2 as "other error" code from probe_flash,
while -1 remains as "no chips found".
Both libflashrom probe APIs v1 and v2 are now handling "other error"
from probe_flash and return it to the API callers as was promised in
the documentation.
This also adds a unit test for error code propagation for "no chips
found" error.
Change-Id: I4a271550bea2b36c657c71ce6cb1927082663c3c
Signed-off-by: Anastasia Klimchuk <aklm@flashrom.org>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/88008
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Stefan Reinauer <stefan.reinauer@coreboot.org>
Reviewed-by: Peter Marheine <pmarheine@chromium.org>
Probing v2 can (if requested) go through all flashchips and find
all the matching chip definitions. This is the way cli behaves,
so cli becomes a client of probing v2.
Previously cli and libflashrom had different probing logic, and
different code in different source files.
This patch also adds tests for probing v2.
Testing from the cli:
./flashrom -p dummy:emulate=W25Q128FV -r dump.rom
./flashrom -p dummy:emulate=MX25L6436 -r dump.rom
./flashrom -p dummy:emulate=MX25L6436 -c "MX25L6473E" -r dump.rom
./flashrom -p dummy:emulate=SST25VF032B -E
./flashrom -p dummy:emulate=S25FL128L -r dump.rom
./flashrom -p dummy:emulate=INVALID -r dump.rom
./flashrom -p dummy:emulate=MX25L6436 -c "NONEXISTENT" -r dump.rom
Change-Id: Idfcf377a8071e22028ba98515f08495ed2a6e9f0
Signed-off-by: Anastasia Klimchuk <aklm@flashrom.org>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/87341
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Peter Marheine <pmarheine@chromium.org>
There were no options available to obtain the list of programmers.
The implementation is based on flashrom_supported_flash_chips.
Arrays of constant strings are returned, and the array must be
freed using flashrom_data_free.
Testing: Both unit tests and CLI tools serve as libflashrom clients.
All unit tests run successfully.
Change-Id: Ib5275b742b849183b1fe701900040fee369a1d78
Signed-off-by: Dmitry Zhadinets <dzhadinets@gmail.com>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/86921
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Peter Marheine <pmarheine@chromium.org>
Reviewed-by: Anastasia Klimchuk <aklm@chromium.org>
Before this commit, any message from Flashrom would trigger
the user's callback. This could lead to additional delays
and slow down overall Flashrom performance.
This patch adds the ability to configure the log level for
messages from Flashrom. It sets the default log level to INFO
Testing: Both unit tests and CLI tools serve as libflashrom clients.
All unit tests run successfully.
Change-Id: I095d48b8feb5fbc950a36eb17bed0d7cb8d9df64
Signed-off-by: Dmitry Zhadinets <dzhadinets@gmail.com>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/87047
Reviewed-by: Anastasia Klimchuk <aklm@chromium.org>
Reviewed-by: Peter Marheine <pmarheine@chromium.org>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
The initial implementation does not account for user_data, requiring
the calling application to use a global scope. This may lead to issues
related to object lifecycle management and other architectural
concerns.
This patch adds user_data to the user’s log callback. Moreover, it
performs message formatting, so the application only needs to pass
the formatted string to the selected output.
The change does not break the existing logging API but extends it.
A new API version is introduced with the v2 suffix.
Testing: Both unit tests and CLI tools serve as libflashrom clients.
All unit tests run successfully.
Change-Id: Iea738bd371fa3d69b9cf222c89ee67490d30af39
Signed-off-by: Dmitry Zhadinets <dzhadinets@gmail.com>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/86875
Reviewed-by: Anastasia Klimchuk <aklm@chromium.org>
Reviewed-by: Peter Marheine <pmarheine@chromium.org>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
The patch updates calculation of total length for the operation
which is displayed with progress.
The reason is: even if, for example the whole chip erase or write
was requested, the actual length of bytes modified can be less than
whole chip size (areas which already have expected content,
are skipped).
Change-Id: I88ac4d40f1b6ccc1636b1efb690d8d68bdebec08
Co-developed-by: Anastasia Klimchuk <aklm@flashrom.org>
Co-developed-by: Sergii Dmytruk <sergii.dmytruk@3mdeb.com>
Signed-off-by: Anastasia Klimchuk <aklm@flashrom.org>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/84439
Reviewed-by: Sergii Dmytruk <sergii.dmytruk@3mdeb.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Original progress reporting implemented in CB:49643 and it has some
issues, for example:
size_t start_address = start;
size_t end_address = len - start;
End address is anything but length minus start address.
update_progress(flash,
FLASHROM_PROGRESS_READ,
/*current*/ start - start_address + to_read,
/*total*/ end_address);
Total should just be length if that's how current value is computed.
---
libflashrom needs to know total size ahead of time.
That's init_progress() and changed update_progress().
It also needs to store the last current value to be able to update it.
That's stage_progress in flashrom_flashctx.
Measuring accurately amount of data which will be read/erased/written
isn't easy because things can be skipped as optimizations. The next
patch in the chain aims to address this, there are TODO/FIXME
comments there.
---
CLI shares terminal with the rest of the code and has to maintain more
state to handle that reasonably well.
Similar to CB:64668, an effort is made to keep the progress on a
single line. Non-progress output is kept track of to know when
moving to a new line cannot be avoided.
---
A script to test the CLI:
\#!/bin/bash
t=${1:-rewW}
shift
if [[ $t =~ r ]]; then
echo ">>> READ"
./flashrom -p dummy:emulate=W25Q128FV,freq=64mhz -r dump.rom --progress "$@"
echo
fi
if [[ $t =~ e ]]; then
echo ">>> ERASE"
./flashrom -p dummy:emulate=W25Q128FV,freq=64mhz -E --progress "$@"
echo
fi
if [[ $t =~ w ]]; then
echo ">>> WRITE (without erase)"
dd if=/dev/zero of=zero.rom bs=1M count=16 2> /dev/null
./flashrom -p dummy:emulate=W25Q128FV,freq=64mhz -w zero.rom --progress "$@"
echo
fi
if [[ $t =~ W ]]; then
echo ">>> WRITE (with erase)"
dd if=/dev/zero of=zero.rom bs=1M count=16 2> /dev/null
dd if=/dev/random of=random.rom bs=1M count=16 2> /dev/null
./flashrom -p dummy:emulate=W25Q128FV,freq=64mhz,image=random.rom -w zero.rom --progress "$@"
echo
fi
Co-developed-by: Anastasia Klimchuk <aklm@flashrom.org>
Co-developed-by: Sergii Dmytruk <sergii.dmytruk@3mdeb.com>
Change-Id: If1e40fc97f443c4f0c0501cef11cff1f3f84c051
Signed-off-by: Sergii Dmytruk <sergii.dmytruk@3mdeb.com>
Signed-off-by: Anastasia Klimchuk <aklm@flashrom.org>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/84102
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Anastasia Klimchuk <aklm@chromium.org>
Without the header being explicitly included, scan-build run on CI
was returning errors for these files, such as:
include the header <stdlib.h> or explicitly provide a declaration
for 'calloc'
The functions in question were calloc, free, etc.
Change-Id: I4b79c5f86c074c456533296c309293e4064abe3f
Signed-off-by: Anastasia Klimchuk <aklm@flashrom.org>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/84455
Reviewed-by: Stefan Reinauer <stefan.reinauer@coreboot.org>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
When flash regions are protected, erase could incorrectly erase regions
which were meant to be protected by requesting the correct size but
using an erase opcode with coarser granularity than desired (for
instance using a 16-byte erase command while attempting to erase only 8
bytes).
This fixes that by exchanging the nesting of the loops over erase blocks
and flash regions.
Old:
- Select erasefns
- Loop over blocks to erase for each selected erasefn
- Loop over programmer flash regions within erase block
- Erase regions (may fail since selected erasefn will be
too big if flash region is smaller than erase block)
New:
- Loop over programmer flash regions within erase block
- Select erasefns within programer flash region
- Loop over blocks to erase for each selected erasefn
- Erase regions
Eraser selection and erasing has also been factored out into a helper
function to manage nesting depth.
TEST=New test cases pass, whereas some of them fail without the changes
to erasure_layout.c
BUG=https://ticket.coreboot.org/issues/525
Change-Id: Ic5bf9d0f0e4a94c48d6f6e74e3cb9cccdc7adec9
Co-authored-by: Nikolai Artemiev <nartemiev@google.com>
Co-authored-by: Anastasia Klimchuk <aklm@flashrom.org>
Signed-off-by: Nikolai Artemiev <nartemiev@google.com>
Signed-off-by: Anastasia Klimchuk <aklm@flashrom.org>
Signed-off-by: Peter Marheine <pmarheine@chromium.org>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/82393
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Anastasia Klimchuk <aklm@chromium.org>
As proposed on the mailing list ("RFC: remove the calibrated delay
loop" [1]), this removes the calibrated delay loop and uses OS-based
timing functions for all delays because the calibrated delay loop can
delay for shorter times than intended.
When sleeping this now uses nanosleep() unconditionally, since usleep
was only used on DOS (where DJGPP lacks nanosleep). When busy-looping,
it uses clock_gettime() with CLOCK_MONOTONIC or CLOCK_REALTIME depending
on availability, and gettimeofday() otherwise.
The calibrated delay loop is retained for DOS only, because timer
resolution on DJGPP is only about 50 milliseconds. Since typical delays
in flashrom are around 10 microseconds, using OS timing there would
regress performance by around 500x. The old implementation is reused
with some branches removed based on the knowledge that timer resolution
will not be better than about 50 milliseconds.
Tested by reading and writing flash on several Intel and AMD systems:
* Lenovo P920 (Intel C620, read/verify only)
* "nissa" chromebook (Intel Alder Lake-N)
* "zork" chromebook (AMD Zen+)
[1]: https://mail.coreboot.org/hyperkitty/list/flashrom@flashrom.org/thread/HFH6UHPAKA4JDL4YKPSQPO72KXSSRGME/
Signed-off-by: Peter Marheine <pmarheine@chromium.org>
Change-Id: I7ac5450d194a475143698d65d64d8bcd2fd25e3f
Reviewed-on: https://review.coreboot.org/c/flashrom/+/81545
Reviewed-by: Anastasia Klimchuk <aklm@chromium.org>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Brian Norris <briannorris@chromium.org>
The test checks that algorithm for erase functions selection
works and there are no regressions.
Specifically, test contains an array of test cases. Each case
initialises a given initial state of the memory for the mock chip,
and layout regions on the chip, and then performs erase and write
operations.
At the end of operation, test asserts the following:
- the state of mock chip memory is as expected, i.e. properly erased
or written
- erase blocks are invoked in expected order and expected number
of them
- chip operation (erase or write) returned 0.
Change-Id: I8f3fdefb76e71f6f8dc295d9dead5f38642aace7
Signed-off-by: Anastasia Klimchuk <aklm@flashrom.org>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/67535
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Peter Marheine <pmarheine@chromium.org>
Some devices such as the GSC knows how it is wired to AP and EC flash
chips, and can be told which specific device to talk to. Other devices
such as Servo Micro and HyperDebug are generic, and do not know how they
are wired, the caller is responsible for first configure the appropriate
MUXes or buffers, and then tell the debugger which port to use (Servo
Micro has just one SPI port, HyperDebug is the first that has multiple).
The Raiden protocol allows both the cases of USB devices knowing their
wiring and not.
If I were to declare the protocol in Rust, this is how the information
of the Raiden protocol "enable request" would be encoded:
```
enum {
EnableGeneric(u8),
EnableAp,
EnableEc,
...
}
```
The first label `EnableGeneric(u8)` is to be used with HyperDebug that
does not know how its ports are wired, and allow access by index.
The other labels `EnableAp` and `EnableEc` are to be used with the GSC.
The actual transmission of the enum above uses the bRequest and low byte
of wValue of a USB control request, but that is a detail and not
conceptually important.
Until now, `-p raiden_debug_spi:target=AP` or `...:target=EC` could be
used to make flashrom use `EnableAp` or `EnableEc`, and if neither was
given, it would default to `EnableGeneric`, which now that wValue is
used means `EnableGeneric(0)`.
I find it rather straight-forward, that `-p raiden_debug_spi:target=1`,
`...:target=2`, etc. should translate to `EnableGeneric(1)`, etc.
This patchset achieves this, by adding a second 16-bit parameter value,
next to request_enable.
I have tested that flashrom can detect the same Winbond flash chip
"W25Q128.V..M" with two different Raiden USB devices as below.
TEST=flashrom -p raiden_debug_spi:serial=0701B044-91AC3132,target=AP
TEST=flashrom -p raiden_debug_spi:serial=205635783236,target=1
Signed-off-by: Jes B. Klinke <jbk@chromium.org>
Change-Id: I03bf4f3210186fb5937b42e298761907b03e08b7
Reviewed-on: https://review.coreboot.org/c/flashrom/+/77999
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Anastasia Klimchuk <aklm@chromium.org>
The main purpose of this patch is to run unit tests on BSD family
of OSes. The root cause is `fileno` syscall which is a macro that
can be expanded to either a function call (for multi-threaded
environment) or to inline code (for single-threaded environment).
Said inline code accesses private field of file descriptor, and
this construction is impossible to mock in unit tests. Multi-
threaded environment has `fileno` as a function, which can be
mocked in unit tests.
On other OSes the patch just creates a thread which is doing nothing.
We avoid adding pre-processor conditionals since the cost is small.
Tested on
FreeBSD 13.1-RELEASE-p6 GENERIC amd64
NetBSD 9.2 (GENERIC) amd64
OpenBSD 7.2 GENERIC#7 amd64
DragonFly v6.4.0-RELEASE x86_64
Ubuntu 22.04.1 x86_64
Change-Id: I3d65c125183e60037ad07b9d54b8fffdece5a4e8
Signed-off-by: Anastasia Klimchuk <aklm@chromium.org>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/74157
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Peter Marheine <pmarheine@chromium.org>
Reviewed-by: Thomas Heijligen <src@posteo.de>
Patch fixes the error:
error: second argument to 'va_arg' is of promotable type 'mode_t'
(aka 'unsigned short'); this va_arg has undefined behavior because
arguments will be promoted to 'int' [-Werror,-Wvarargs]
Discovered and tested on:
FreeBSD clang version 13.0.0
gcc 8.3.0 "cc 8.3 [DragonFly] Release/2019-02-22"
Also tested on:
gcc 11.3.0 "cc (Ubuntu 11.3.0-1ubuntu1~22.04) 11.3.0"
Change-Id: I95b7c8dafdf4e7664c48a952acd7f8eaedb59ba7
Signed-off-by: Anastasia Klimchuk <aklm@chromium.org>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/74202
Reviewed-by: Peter Marheine <pmarheine@chromium.org>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Thomas Heijligen <src@posteo.de>
Add a new wp_cfg_to_reg_values() function that takes a generic wp_cfg
instance and returns the chip-specific values that need to be written to
the chip's registers to enable the specified protection range/mode.
The function returns three values for each chip register:
- reg_values[reg] - Value writeprotect will write to reg
- bit_masks[reg] - Bit mask for WP-related bits in reg
- write_masks[reg] - Bit mask for writable WP-related bits in reg
(i.e. the ones writeprotect will try to write)
BUG=b:260019525,b:259013033,260020006
BRANCH=none
TEST=ninja test
Change-Id: Ib2a47153b230c9f82bb4eca357c335f2abbacc20
Signed-off-by: Nikolai Artemiev <nartemiev@google.com>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/69847
Reviewed-by: Edward O'Callaghan <quasisec@chromium.org>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Sergii Dmytruk <sergii.dmytruk@3mdeb.com>
Dummy programmer has a shared data between *_masters. To make sure the
dummy has no memory leakage, we need a test that will covers
initialization and shutdown of the programmer with different bus
types, i.e. programmer specific, non-SPI and SPI.
TEST=ninja test
Change-Id: Iafe715feb5f5c0b5efd6827cdb2c3a314f542319
Signed-off-by: Alexander Goncharov <chat@joursoir.net>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/72665
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Anastasia Klimchuk <aklm@chromium.org>
A written region that is sized below that of the erasure granularity
can result in a incorrectly read region that does not include prior
content within the region before the write op. This was dealt with
in ChromeOS downstream by expanding out the read to match the erase
granularity however does not seem to impact upstream. Add a unit-test
to avoid regression as this is important behaviour to cover.
Change-Id: Id3ce5cd1936f0f348d34a6c77cee15e27a5c353f
Signed-off-by: Edward O'Callaghan <quasisec@google.com>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/71659
Reviewed-by: Sam McNally <sammc@google.com>
Reviewed-by: Evan Benn <evanbenn@google.com>
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=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>
Anastasia Klimchuk
Dmitry Zhadinets
Simon Arlott
Peter Marheine
Anastasia Klimchuk
Jes B. Klinke
Nikolai Artemiev
Alexander Goncharov
Edward O'Callaghan
Evan Benn