[fuse_ctrl,dv] Split up smoke_test_fc_ratchet_seed_write_read#756
Closed
rswarbrick wants to merge 9 commits intomainfrom
Closed
[fuse_ctrl,dv] Split up smoke_test_fc_ratchet_seed_write_read#756rswarbrick wants to merge 9 commits intomainfrom
rswarbrick wants to merge 9 commits intomainfrom
Conversation
This avoids fuse_ctrl_mmap.h defining something that actually gets instantiated (and defines a symbol). Without fixing that, if more than one compilation unit includes the header then link will fail because more than one object file defines "partitions" and various fuse index arrays.
No functional change (but hopefully it makes things a bit clearer: I found these ranges a bit hard to reason about).
The value in caliptra_ss_fuse_ctrl_manuf_prod_prov hadn't yet been updated for the change in 275521b. This change should make it so that test and also caliptra_ss_fuse_ctrl_init_fail get a magic number from a single (documented) place. Apply the same change to caliptra_ss_fuse_ctrl_test_unlocked0_prov.
This is a bit fiddly to do properly, because you end up needing to map from a partition with a read lock to that partition's read lock address (which depends on soc_mmap.h). My solution is to have a template generate that map (which will contain actual addresses that get pulled from soc_mmap.h at compile time).
Carefully work through the logic in wait_dai_op_idle and document exactly what it is doing. This changes behaviour slightly if status_mask is nonzero. To look at all sites where this might have an effect, I grepped for the following regex: "wait_dai_op_idle([^0)]" This commit tidies up the obvious matches. There are a couple of places where this function is copy-pasted: - `cptra_uds_prog.c` - `cptra_zeroization` Tidying these up to use the (now correct!) version of this function should be a separate change.
This test injects an error and then reads the status register to check it shows a bus integrity error. The preceding dai_wr transaction is wrong though: *that* should report the error (by reading the same register). Apparently, that check wasn't working before but we've just tweaked wait_dai_op_idle to check more sensibly and that now fails because we're calling it with no expected error. Expect the error, which fixes that failure and also makes the following read redundant.
These are "safe" to add, because no caller expected a return value before! The advantage is that tests that want to do some DAI operations can fail rather more quickly if something unexpected happens.
This test was taking a long time (more than 4 hours) and thus got killed. But this is a bit silly: we don't really expect the partitions to interact with each other anyway. I've made lots of tidy-ups in the code and made things complete more quickly by just checking two partitions each time. Of course, that lowers the test coverage (because 2 < 8). To get that back again, I've multiplied the reseed count for the test by 5 (which is more than 8/2).
…ith updated timestamp and hash after successful run
Contributor
Author
|
Now part of #774. |
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
1 participant
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.
To avoid a bunch of PRs that all depend on each other, this builds on #751. The last two commits are unique to this PR. The first adds some status reporting to the DAI functions and the second is the main point of the PR: it splits up this test, which was timing out.
Honestly, I think we could also reasonably only write some random subset of the fuses in each partition (since we don't really expect variation within a partition). But I'm hopeful that this will already be split up enough: I think the previous version of the test wasn't quite finishing in 4 hours. Dividing by 4 should hopefully mean it now finishes in roughly an hour.