`run_init_error_path` tests the scenario when init function of
a programmer fails. Init can fail at different phases and depending
on a specific test scenario it could be before or after shutdown
function has been registered.
If shutdown function has already been registered, it needs to run
because it cleans up the resources allocated during init. This patch
prevents memory leaks.
BUG=b:181803212
TEST=ninja test
Change-Id: I604edff18e35b7c044b73d3a8adfa8c800eddfd2
Signed-off-by: Anastasia Klimchuk <aklm@chromium.org>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/67198
Reviewed-by: Edward O'Callaghan <quasisec@chromium.org>
Reviewed-by: Felix Singer <felixsinger@posteo.net>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Previously, parade lspcon unit test had no custom emulation and
was running basic lifecycle with default mocks (default mocks do
nothing and return success). I have discovered that it does not
work in all environments.
Specifically, in functions `parade_lspcon_wait_command_done`
and `parade_lspcon_wait_rom_free` there is a local variable
`uint8_t val` which is declared and not explicitly initialised. It can
get different initial values depending on the environment/toolchain
(for example gcc vs clang).
Later during the code execution, this variable is used as a holder
for writing/reading data from register(s). For unit test, reading and
writing data from registers is emulated. With default mocks, initial
value of variable was propagated further as if it was "read" from
a register. So the unit test could pass or fail depending on the
initial value of local variable, which in turn depends on environment
and toolchain.
If initial value was 0 the test reliably passed (this is the case with
upstream build environment, locally I have gcc 11.3.0 on x86_64).
If it was 1 the test reliably failed (this is the case of chromium
flashrom tree, this is clang 15.0 under chroot, tested on x86_64 and
arm boards).
If it was any other value then it could be anything.
This patch adds custom mocks for ioctl/read/write operations
and emulates a successful scenario of running a lifecycle. Local
variable is initialised by reading from the register, same as it
happens in the real (non-test) run for this programmer.
BUG=b:242816982
TEST=ninja test
Change-Id: I98f52507a0ddbbfbeb390038d14192cacc2be683
Signed-off-by: Anastasia Klimchuk <aklm@chromium.org>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/67161
Reviewed-by: Peter Marheine <pmarheine@chromium.org>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Felix Singer <felixsinger@posteo.net>
These functions [isspace, etc] check whether c [(the argument)], which
must have the value of an unsigned char or EOF, falls into a certain
character class according to the current locale.
Cast the argument from `char` to `unsigned char` to guarantee that
we don't pass illegal negative values. Some implementations actually
provide a warning to get heads up[1].
[1] https://man.netbsd.org/ctype.3#CAVEATS
TEST=Builds on Cygwin (Windows 10, amd64, gcc 11.3.0)
Change-Id: Ia48d5a19b0964bc28e5360edf06bdf287dad5945
Signed-off-by: Thomas Heijligen <thomas.heijligen@secunet.com>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/66548
Reviewed-by: Nico Huber <nico.h@gmx.de>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-by: Anastasia Klimchuk <aklm@chromium.org>
Instead of using reversible write (rpci_write_long) that relies on
global state, do it manually. Save original PCI config space
register contents to programmer's structure during initialization
and restore it in programmer's shutdown.
TOPIC=reduce_global_pci_state
TEST=builds
Change-Id: I66c55daecf15b24b52aef33b14ff6746349cfb8c
Signed-off-by: Alexander Goncharov <chat@joursoir.net>
Ticket: https://ticket.coreboot.org/issues/389
Reviewed-on: https://review.coreboot.org/c/flashrom/+/66857
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Felix Singer <felixsinger@posteo.net>
flashrom_tester 'flashrom' crate was implemented using the flashrom
commandline. Add a second implementation using the libflashrom interface
via the libflashrom and libflashrom-sys rust bindings.
BUG=b:230545739
BRANCH=None
TEST=cargo test
TEST=on grunt (AMD)
TEST=/usr/bin/flashrom_tester --libflashrom host
TEST=/usr/bin/flashrom_tester --flashrom_binary /usr/sbin/flashrom host
Change-Id: Ic4db6c829d7e8dc707a10c10e1ca0d9b8abccdec
Signed-off-by: Evan Benn <evanbenn@chromium.org>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/65282
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Edward O'Callaghan <quasisec@chromium.org>
This is analogous to spi.c and opaque.c however parallel
logic was previously never consoldiated.
This free's up flashrom.c from namespace pollution.
BUG=b:242246291
TEST=builds with both make and meson.
Change-Id: Ie08e2e6c51ccef5281386bf7e3df439b91573974
Signed-off-by: Edward O'Callaghan <quasisec@google.com>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/66651
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Thomas Heijligen <src@posteo.de>
Constructing a written file from a buffer is auxiliary
functionality to the core flashrom algorithms. Move
aside to decrease the overall complexity of flashrom.c
BUG=b:242246291
TEST=builds
Change-Id: Ib613e74597d4bdd689043ba93aeb6a87ec80cc14
Signed-off-by: Edward O'Callaghan <quasisec@google.com>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/66646
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Evan Benn <evanbenn@google.com>
Reviewed-by: Sam McNally <sammc@google.com>
Reviewed-by: Thomas Heijligen <src@posteo.de>
The hwseq_data struct remains to be a stack allocated
instance for now however it is referenced in the opaque_master
data field of the flash ctx. This allows for worker functions
that leverage hwseq_data state to derive state indirectly via
the flash ctx instead of directly upon the global state.
This allows for unblocking further work to finally dispense
with the stack allocation in favour of a heap allocation with
a defined life-time of the driver.
BUG=b:237839418
TEST=builds
Change-Id: I2ad57d496176cf26edcd0ba40154cc6250846e33
Signed-off-by: Edward O'Callaghan <quasisec@google.com>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/65205
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Anastasia Klimchuk <aklm@chromium.org>
Reviewed-by: Alexander Goncharov <chat@joursoir.net>
Reviewed-by: Thomas Heijligen <src@posteo.de>
Provide the meson option `-Dich_descriptors_tool=auto/enable/disabled`
to determin if the ich_descriptors_tool should be build or not. On
`auto` or `enabled` it will be, on `disabled` it will not be build.
This is usefull for environments where the ich_descriptors_tool is not
needed.
Change-Id: Ief65a914019f72536d563eda36bd7f2f5330bea8
Signed-off-by: Thomas Heijligen <thomas.heijligen@secunet.com>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/66704
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Edward O'Callaghan <quasisec@chromium.org>
crossystem uses flashrom to gather data on some platforms. To avoid
firmware lock deadlock, call crossystem before initialising libflashrom.
When querying hardware write protect status, provide an argument to
crossystem so that only that field is queried. This also avoids the
deadlock, and improves performance.
BUG=b:239496316
BRANCH=None
TEST=on trogdor(arm), grunt(amd), hatch(intel):
TEST=flashrom_tester --libflashrom /usr/sbin/flashrom host Coreboot_ELOG_sanity
TEST=flashrom_tester /usr/sbin/flashrom host Coreboot_ELOG_sanity
Change-Id: I7d94cfc6ccbfbec91f12151eb0004724ccfc4e00
Signed-off-by: Evan Benn <evanbenn@chromium.org>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/65962
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Edward O'Callaghan <quasisec@chromium.org>
Instead of using reversible write (rpci_write_word) that relies on
global state, do it manually. Save original PCI config space
register contents to programmer's structure during initialization
and restore it in programmer's shutdown.
TOPIC=reduce_global_pci_state
TEST=builds
Change-Id: Ie99a7cb3e049b29e6838dd4af587eee8c3b18919
Signed-off-by: Alexander Goncharov <chat@joursoir.net>
Ticket: https://ticket.coreboot.org/issues/389
Reviewed-on: https://review.coreboot.org/c/flashrom/+/66838
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Nico Huber <nico.h@gmx.de>
Reviewed-by: Felix Singer <felixsinger@posteo.net>
Add an explanation of the two rust libraries, and instructions for
building. Note that the bindings are not included in the tarball.
BUG=None
BRANCH=None
TEST=None
Change-Id: I05b1b5821554f8faee7728f899d51b6e8e9bf5be
Signed-off-by: Evan Benn <evanbenn@chromium.org>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/66618
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Edward O'Callaghan <quasisec@chromium.org>
Write a newline after the hardware write protect prompt. Automated tests
read stdout and wait for this message, and split on newline, so write a
newline.
Also modify the function to not be recursive. Try to handle a closed
input correctly - panicing in that case. Behaviour is now to wait for
a newline instead of for 1 character.
BUG=b:240512896
BRANCH=None
TEST=tast run localhost:2222 firmware.FlashromTester
TEST=flashrom_tester < /dev/null
TEST=flashrom_tester; type some things, hold enter, then close stdin
Change-Id: I07ec242ca0d41787030d5d27fc88d78ed884d746
Signed-off-by: Evan Benn <evanbenn@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/third_party/flashrom/+/3809595
Reviewed-by: Nikolai Artemiev <nartemiev@google.com>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/66587
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-by: Anastasia Klimchuk <aklm@chromium.org>
Reviewed-by: Edward O'Callaghan <quasisec@chromium.org>
Depend exclusive on the cmocka package provided by the build
environment. Cmocka is widely available in Linux distributions and BSD
systems. Besides that, it is suboptimal to fetch own dependencies in the
build process. Most packaging systems even forbid fetching additional
assets at build time.
Change-Id: I751c85d5f72e47356113cf55dfbaec73cbd8028c
Signed-off-by: Thomas Heijligen <thomas.heijligen@secunet.com>
Reviewed-on: https://review.coreboot.org/c/flashrom/+/66696
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-by: Edward O'Callaghan <quasisec@chromium.org>
Reviewed-by: Anastasia Klimchuk <aklm@chromium.org>
Reviewed-by: Felix Singer <felixsinger@posteo.net>
Currently, each programmer parameter has their own temp variable to
store their raw value into it. That's not needed since these variables
are only used for a short time to do some configuration and stay unused
then. Thus, use only one variable for all of them.
Signed-off-by: Felix Singer <felixsinger@posteo.net>
Change-Id: Ib4ebc0e6354aad007145e1b0a761d9011c59ff7c
Reviewed-on: https://review.coreboot.org/c/flashrom/+/66571
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Thomas Heijligen <src@posteo.de>
Currently, each programmer parameter has their own temp variable to
store their raw value into it. That's not needed since these variables
are only used for a short time to do some configuration and stay unused
then. Thus, use only one variable for all of them.
Signed-off-by: Felix Singer <felixsinger@posteo.net>
Change-Id: I31e0baa2c5800c722a9ba853bcd40d71ed343f6d
Reviewed-on: https://review.coreboot.org/c/flashrom/+/66568
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Thomas Heijligen <src@posteo.de>
