[test,do_not_merge] Test FPGA loading changes against CI#30504
Draft
AlexJones0 wants to merge 26 commits into
Draft
[test,do_not_merge] Test FPGA loading changes against CI#30504AlexJones0 wants to merge 26 commits into
AlexJones0 wants to merge 26 commits into
Conversation
All ROM_EXT end-to-end ownership tests use the same common initialization logic for clearing the bitstream - refactor these tests to instead declare a dictionary of params, which will have the test command automatically prepended by the ownership test definition. Importantly, this will allow us to more easily add shared fpga_params between all the different ownership tests later on, and will help to reduce duplication. This is a partial port of d2c06ac. This commit was originally motivated for QEMU changes - but QEMU is not relevant currently on `master`, whereas we do need the deduplication for the addition of parameters to control bitstream clearing etc. Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
Currently, an execution environment can derive from a base execution environment. However, some fields, like `param`, are dictionaries are completely replaced instead of partially overridable. This change introduces `_NESTED_FIELDS` to indicate fields that should merge their values when derived from a base, rather than just replacing them. Specifically, it enables `param` to be partially overridden, allowing a derived environment to add or modify specific parameters without losing all parameters from its base. Change-Id: Ie0922e2e90c80c92749907f491109b527879a278 Signed-off-by: Yi-Hsuan Deng <yhdeng@google.com> (cherry picked from commit 965509b)
This change extracts the FPGA test command setup and cleanup logic into a more modular system using `testopt` flags, which reduces the boilerplate needed to customize test commands. Change-Id: I008cf6ce4617ba5afe2adf81df855e8593d4995e Signed-off-by: Yi-Hsuan Deng <yhdeng@google.com> (cherry picked from commit 1f5acbb)
Added the `drop-reset` command line option to `opentitantool` and `opentitanlib`, which allows de-asserting the `RESET` pin before executing any command. This is useful for devices held in a reset state, where `RESET` needs to be de-asserted to allow the host to communicate with the device. Change-Id: I7c9b3520132f635d9a12733e5e22fe658f6961a3 Signed-off-by: Yi-Hsuan Deng <yhdeng@google.com> (cherry picked from commit f541a5d)
This change: * Cleans up FPGA parameter fields by utilizing the new partial overwrite semantics. * Sets up FPGA execution environments using `testopt` flags. * Modifies the default `test_cmd` for FPGA environments to be empty, providing a better default for custom harnesses that don't require flags. Change-Id: I645ac75eb2007d0d109bcab2fdc0cad042e7c8e1 Signed-off-by: Yi-Hsuan Deng <yhdeng@google.com> (cherry picked from commit 70fb7ef)
The new testopt FPGA test parameters are not necessarily backwards compatible with the existing tests. To ensure that all tests that are running in CI continue to pass after these changes, we must tag tests that do not wish to perform any bootstrapping as such (instead of using the test_cmd / clear bitstream). Partially based on the changes in 4e303bd on the `earlgrey_1.0.0` branch. Co-authored-by: Yi-Hsuan Deng <yhdeng@google.com> Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
Apply the new Bazel testopts for OpenTitan tests to make sure that no custom/manual test commands refer to "fpga load-bitstream" or "fpga clear-bitstream" - all bitstream loading / clearing etc. should go through the new testopts interface now. Partially based on the changes in 4e303bd on the `earlgrey_1.0.0` branch. Co-authored-by: Yi-Hsuan Deng <yhdeng@google.com> Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
This tag (and comment) are now stale - on `master`, the CW310 execution environment uses the Hyperdebug-variant "hyper310" exec_env / interface by default, which as the comment suggests is more realistic in terms of a provisioning scenario. Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
The current HW configuration of the bkdr_loader always sees the `target_idx` as being misconfigured, causing every write operation issued to instead be executed as a read - preventing use of the IP for its primary purpose. The `BkdrValidTgts` array was configured as a packed array instead of an unpacked array. As a result, the `inside` operation treated the entire packed array as a single contiguous value (and thus the `target_idx` was never "inside" the array, and the target never valid). By changing this to an unpacked array, `inside` now treats the array elements as individual choices for a match and works as intended. Also ensures the proper enumerated type is being used in the comparison itself, and removes the extraneous zero-extension of the literals assigned to the various enumerated constants. Co-authored-by: Thomas Benz <tbenz@lowrisc.org> Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
The `STATUS` register description was not correct compared to the actual implemented HW behaviour. Slightly extended the `DONE` description to clarify its usage and impact. Also, in the `README.md`, this commit modifies the formatting to match standard OT Markdown styling with one sentence per line. Remove the regression links since they don't make sense for this IP. Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
This commit moves the BKDR_LOADER interface docs to go through cmdgen.py as other IP blocks do, and also updates some parameter documentation that appears to be out of date when comparing to the RTL/HJSON constants. Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
The existing VMEM implementation assumed that VMEMs would always contain 32 bit words, and that we always want to interpret the VMEM with byte address strides. This is true for the existing use case of SRAM programming, but will not be true if we want to load other VMEMs onto the FPGA such as the scrambled ROM or OTP configurations, which have 39- and 22-bit word sizes respectively and both use word address strides in their VMEM files. Update the VMEM model and VMEM parsing logic so that we now store Vec<Word> instead of Vec<u32>, where a Word is essentially just a Vec<u8> of bytes stored in Big Endian order. Likewise, make VMEM parsing and addressing logic consume an address stride argument, so that we know how we should traverse VMEM addresses when e.g. merging sections or enumerating the various data-address pairs. Finally, this changes the existing VMEM consumer (load_sram_program) to make use of these changes. Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
Add VMEM parsing tests related to the newly introduced address stride and arbitrary sized word features. Also extend existing test coverage to features that were not previously tested, such as the merging of contiguous VMEM sections. Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
When we're doing lots of DMI operations, these become very noisy. They make much more sense to exist at a "Debug" logging level. Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
This commit implements the initial logic for integrating the FPGA backdoor loader into OpenTitanLib & OpenTitanTool. It provides interfaces for connecting to the bkdr_loader TAP via OpenOCD, entering the backdoor loader (applying the TAP strapping and resetting), and for enumerating/discovering the target information from the CSRs. A new `opentitantool bkdr` command is implemented for using this functionality, with 3 subcommands - `enter` to enter the backdoor loader, `exit` to finish using it and take OT out of reset, and `info` to list info about the available targets (or a specific target). As part of this, there is some logic for looking up a target by its unique ID. Note that we return handles for each of the different targets which simply wrap the `BackdoorTargetInfo` - these will be extended in future commits to allow operations on the various targets. Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
Add a `bkdr read` command to OpenTitanTool / OpenTitanLib which can be used to read from target memories via the backdoor loader (when it is in its preloading mode, entered via `bkdr enter`). Options are available for customizing the offset & size of the read, as well as to output the data in raw/hex/VMEM formats. Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
This behaviour can technically be replicated by first using `opentitantool fpga clear-bitstream` and then `opentitantool fpga load-bitstream`, but this is less convenient - this new `--force` option acts as a convenience helper. This is more important when using the FPGA bkdr_loader flow which can change the FPGA memory contents without changing the bitstream USR_ACCESS value. Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
This commit introduces a `batched_dmi_writes` method to the `DMI` trait, which implementers can override to provide optimized implementations that batch multiple sequential DMI writes, without caring about reading out the returned values. This will be a particular pain point for interaction with the DMI in the FPGA bkdr_loader, and as such optimizing these writes can potentially give us a 5x speedup by reducing the number of JTAG transactions and the amount of idle time spent waiting for results (i.e. for reducing the overhead of each write). Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
Add a `bkdr write` command to OpenTitanTool / OpenTitanLib which can be used to write to target memories via the backdoor loader (when it is in its preloading mode, entered via `bkdr enter`). Options are available for customizing the offset and read and whether readbacks & status checks are implemented for verifying the write (which takes much longer, due to the various optimizations that are possible when writing but not when reading). 3 different input formats are supported - hex (as args to the command), clear (clear some number of words with a given repeated byte pattern), and vmem (as a path to some VMEM file). This should be enough to support a variety of use cases - writing VMEMs of the scrambled ROM and OTP images, clearing some target's memory, and manually modifying specific memory values via fine-grained commands. A `bkdr verify` command is added - this is basically the same as `bkdr write --verify`, but without doing the actual write operation. The intention is that you can read and verify that the contents of some memory matches your input, without needing to perform any write operation beforehand, or deal with the manual comparison logic yourself outside of OpenTitanTool after reading. Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
Adds a `bkdr batch` command to OpenTitanTool / OpenTitanLib which can be used to batch multiple write operations to target memories via the backdoor loader (when it is in its preloading mode, entered via `bkdr enter`). This command exists despite the presence of `opentitantool --exec` to reduce OpenOCD overhead from repeated writes. We may want to load many images onto the FPGA (e.g. ROM, OTP, flash), and for each opentitantool command a new instance of OpenOCD will be spawned and connected to the backdoor TAP. Rather than incur this heavy overhead, we can spawn OpenOCD once and batch together all of the writes. Due to the added complexity of dealing with nested/chained sub-commands, a limited subset of write options are supported here (only VMEM formats for now). You can still choose to `--verify`, but this will apply to all writes that are completed. There is also a `--start` option which will perform `bkdr exit` for you, since that command would also require an additional OpenOCD instantiation which we would like to eliminate. Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
With appropriate supporting infrastructure in place, this endpoint can now be enabled once more. Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
Still very WIP, needs to be cleaned up, split up etc. Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
AlexJones0
force-pushed
the
fpga_load_testing
branch
3 times, most recently
from
ec02316 to
b023acd
Compare
Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
AlexJones0
force-pushed
the
fpga_load_testing
branch
from
b023acd to
72e4a7d
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR is a Work-In-Progress and is being used to test changes to the FPGA flow against CI. Do NOT merge this PR.
Replace the FPGA bitstream splicing flow (via
updatemem) with a backdoor loading mechanism.Note: this is currently expected to be quite slow (~7s for ROM & OTP splicing on every test) because the loading goes through JTAG operations to a DMI via the bkdr TAP, meaning each write requires individual
drscanoperations to the loader's write data registers. It also currently goes via CMSIS-DAP, which is limited to just over 1000 kHz JTAG adaptor clock speed. Ideally in the future we will migrate to primarily using the FTDI adaptor (now that only the CW340 is supported onmaster), which lets us do up to 15,000 kHz, but that will be a more complicated/involved change due to the necessity of changing CI.Note:
USR_ACCESSstill goes via the ROM (and Ibex) instead of using thebkdr_loaderblock.