[rtl] FPGA bkdr fix#30680
Draft
gautschimi wants to merge 25 commits into
Draft
Conversation
Previously, we pruned anything that has a dependency on the spliced bitstream for the list of software targets to be built. This is because the CI runner does not have support for bitstream splicing, but also acts as a bit of a "catch-all" way of filtering out anything that needs a bitstream, and generally any OpenTitan tests which support FPGA environments - since those will be built in the relevant FPGA jobs anyhow. Since the spliced bitstream will be removed, we should be a bit more explicit about what we're trying to exclude here. For tests, it makes a lot more sense to use appropriate Bazel test tag filters and tags on the test. This will let us catch anything that depends on Verilator, a CW340 bitstream, or QEMU specifically. However, tests that are aliased (e.g. with `opentitan_alias_test`) will not inherit all their parent tags, and will appear as valid build & test targets (to prevent issues with duplicate test appearances when querying). To stop us building these aliased tests, we filter out any targets tagged with 'FPGA', and the set of targets which (transitively) depend on such targets. Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
We're no longer filtering out all OpenTitanTest targets since we removed the filter on the bitstream splice in the CI "Software Build & Test" job - so now we need to be careful to explicitly exclude QEMU from the list of test tag filters. Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
This was a simple shell script that wrapped some Bazel commands for verifying on-target coverage measurement infrastructure. It has been broken in `master` for a while as the `gcp_spliced_test_rom` target does not actually exist, and has instead been replaced by a much more comprehensive coverage infrastructure integrated with our build system & tooling. Thus, remove this deprecated test script. Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
These docs are outdated - the CW310 target has been removed, there is no more "hyper3X0" naming style (Bitstreams are always built for HyperDebug targets), the `--ast-cfg-version` option was removed, and the emulation SKU must be used instead of the SiVal SKU for testing (at least, as the default in the documentation). Also, the `runfiles/sc_hsm/*` pattern for unzipping not needed, as it should be available after unzipping via `runfiles/+hsm+sc_hsm/sc_hsm/`. Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
Since most tests are run in the SiVal ROM_EXT job, the additional overhead that will be added per test by the bkdr_loader flow (when using the CMSIS_DAP adapter) is about 8-10s per test, which causes this job to far exceed the allocated hour. Split this job into two to accommodate the change in runtime (at least, whilst we are using the CMSIS_DAP adapter). Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
We're still getting the cached bitstream from the bucket, but we're not longer splicing it, so update the name to reflect this change. Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
Add rules to allow for retrieving the bitstream and VMEMs (e.g. ROM/OTP) of an execution environment, with the ability to override the file that is provided using `src`. This is intended as a partial replacement for the bitstream splicing flow, which is currently overloaded - it is being used for both bitstream splicing (as the name suggests), and as a hook for build options to be able to modify the OTP and ROM that is spliced in and used for test execution. Bitstreams will no longer be spliced when using the backdoor loader flow, which means the OTP and the ROM can be directly overwritten. However, we still want the option to have build options hook into and overload the ROM or OTP (e.g. on a Bazel transition) - so replace them with these two rules, which targets can use accordingly. Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
When FPGA bitstream splicing is replaced by backdoor loading, the orchestrator will no longer work (as it directly takes as input a spliced bitstream, which it then passes to the test to load). Modify the orchestrator source to package the ROM and OTP files separately into the Orchestrator zip, and to extract and load them when running the CP stage of FPGA provisioning flows. The underlying `bitstream_splice` target will likewise no longer exist after the Bazel transition to the fpga_bkdr loader are merged - as such, we define new targets and build options to transition on, which are just local to the orchestrator now. Also update the docs accordingly. Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
The FPGA Python test binaries have the ability to provide a bitstream. In such cases, the test logic will not work, as the FPGA will also need to be loaded with the ROM and OTP images. Provide the plumbing necessary to pass these as arguments through to opentitantool commands so that we can load these memories via the bkdr_loader on the FPGA. Note that there is no Bazel logic here to actually provide these arguments - that will need to be fixed separately in the future. Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
In preparation for moving from the bitstream splicing flow to the FPGA bkdr_loader flow, we must remove dependencies on the spliced bitstream target. One such target is the dev bundle for releases. Now, it makes more sense to package together the standard CW340 ROM_EXT VMEM images for the ROM and the OTP, so that they can be directly loaded onto a CW340 target via OpenTitanTool. We likewise now package a patched version of OpenOCD in the dev bundle, since this will be needed to load the FPGA memories over JTAG. OpenOCD will include the CMSIS-DAP adapter config by default, but we also include it here for completeness to allow users to make local modifications to the flow. Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
The bkdr_loader re-uses the DFT TAP, so the rv_dm JTAG tap sel has to account for this new logic and include FPGA-specific logic for proceeding through the bkdr_loader after a reset with the TAP strap applied. Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
This doc comment was outdated anyhow, as it referenced the CW310 which has now been deprecated from CI. Update the comment to reflect the original intent, but now updated to suit the fpga bkdr_loader workflow, which is more involved (as you need to build OpenOCD and run OpenTitanTool - there is no more single "packaged" spliced bitstream). Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
With the appropriate supporting infrastructure for the backdoor loader in place, we can now re-enable the ROM endpoint as a target that is exposed via the bkdr_loader interface. Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
Instead of using a bitstream spliced with the ROM and OTP, we always use the same base CW340 bitstream when running on FPGA. The scrambled ROM and OTP images are then loaded over the FPGA-unique bkdr_loader IP. To reduce the difference from the previous behaviour and avoid side effects being built up in NVM (e.g. flash info pages) across test runs, we zero other memories at the start of each test. This importantly encompasses retention SRAM and the flash info pages, but we also clear the flash data pages for good measure, to be as close to the previous spliced bitstream flow as possible. The SRAM of a loaded bitstream would be initialized to some scrambled value, and so we opt not to zero it instead (it is also unlikely that any tests would be meaningfully impacted by state carried across in SRAM). Since the backdoor loader requires JTAG, JTAG is now made a dependency of every test. All other consumers of the spliced bitstream target have been converted, so we now simply remove this target and the logic for splicing the bitstreams, as this will not be supported going forward. Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
Replace the existing MMI file generation logic with some simple commands that write some text to a dummy MMI file. The MMI file is not removed completely at this staged since there is still various pieces of infrastructure (Bazel, CI bitstream cache) that assume it exists. However, it will not be needed as we are now switching from the bitstream splicing flow to the bkdr_loader flow on FPGA, and this logic is in fact complicating the increase of the size of the ROM. As such, for now we stub this out with some simple textual contents. This file should be completely removed in the future. Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
We no longer splice bitstreams in CI, so it no longer makes sense for the `.orig` and `.splice` references to be in the test. Still, it's useful to have the functionality offered by these tests, so we just rename the bitstreams that are being used in this test. Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
These scripts were being used to generate e.g. ROM images that were specifically compatible with the VMEM format that Vivado's Updatemem tool expects. Since we no longer use the bitstream splicing flow and therefore no longer use `updatemem`, this script (and its relevant test & simulator) will no longer be usable with the larger ROM (which will not have an MMI file with the BRAM mapping needed for splicing). As this script will not be useful going forward and has been effectively replaced by the bkdr_loader flow on FPGA, remove it completely. Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
As with many manual test targets, this has become broken over time. Most importantly, we cannot execute in the rom_with_fake_keys environments because these are signed with RMA keys, while we need DEV - and it is not trivial to change the key for a single env. Also remove the stale manual tag that came from when this test previously used a manual bitstream override. This is no longer relevant given the build system (exec_env and FPGA bkdr_loader) improvements. Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
These docs had not yet been updated to note the jumper cables necessary for running the SPI TPM tests. Note them and give an example of a test that can be run to confirm that they have been configured correctly. Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
Correct a couple of outdated parts of these setup docs - particularly with regards to the fact that we do not really support using the CW340 without the Hyperdebug at all. Make the separation between the board and the interface (as given to OpenTitanTool) clearer and note the requirement of Hyperdebug for various parts of the OpenTitan test flow. Then, perform a larger overhaul to cover the new backdoor loading flow, which replaces the old bitstream splicing workflow for setting up the test environment of an FPGA. Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
This documentation was already out of date. Update it to match the new FPGA bkdr_loader flow and remove the references to splicing. Update the example test to match the current configuration, and make note of the current `changes_otp` flag which is used (and why it is used!) Make some small changes to the example of overriding the bitstream since we no longer have any tests that do that - it is still supported, but heavily dissuaded. Also, change the formatting to recommend current OpenTitan style guides (one sentence per line). Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
This `splice_rom` script is outdated and out-of-use, and had been replaced by the Bazel splicing flows. This is now even further replaced by the new FPGA backdoor loading flow, and so the script no longer makes sense to keep, and should be entirely removed. Similarly, the FPGA reference documentation is somewhat out of date and also needs updating for the new flow. Make these changes to ensure that the doc is (relatively) up-to-date. Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
Remove references to splicing bitstreams and update docs where relevant. Also address some smaller areas of documentation that have fallen out of date with the current practices. Where necessary, apply relevant OpenTitan Markdown styling for consistencfy (one sentence per line). Additionally, add a small note for a needed FIXUP with old CW310 references, which should be replaced with CW340 references (but is deferred to a separate commit). Signed-off-by: Alex Jones <alex.jones@lowrisc.org>
Invalid target if target_idx is >= NumBkdrTgts. (instead of first casting from 8b to 4b vector and then checking if inside valid enums) Signed-off-by: Michael Gautschi <mgautschi@lowrisc.org>
The muxing of the jtag was previously dependent on the state. This can lead to glitches after implementation because the state is a multibit signal. Vivado uses one-hot encodings for FSM states. Hence, the bkdr_loader mux signal depends on three registers (despite being declared as a 2b signal). The state transition from PRELOAD to SWITCHING happens right when we write to the done signal and can lead to a glitch if the three state bits don't change at the same time. If it happens on the JTAG clock, it will upset the JTAG statemachine and lead to transmission errors. A simple fix is to register the mux signal. Ideally the mux-signal would be synchronized to the jtag clock domain. But this is tricky because we cannot rely on the JTAG clock being active. The already implemented delayed switching should be sufficient if we can guarantee that the jtag is inactive when the switch is performed. Signed-off-by: Michael Gautschi <mgautschi@lowrisc.org>
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 based on #30543
Only the two last commits are relevant.