[flash_ctrl,dv] Lots of broken block-level tests in flash_ctrl

[rswarbrick]

Hierarchy of regression failure

Block level

Failure Description

The nightly DV runs report lots of failing tests in flash_ctrl. They seem to have started breaking on 28th April, including the following tests:

• flash_ctrl_access_after_disable
• flash_ctrl_integrity
• flash_ctrl_intr_wr
• flash_ctrl_intr_wr_slow_flash
• flash_ctrl_oversize_error
• flash_ctrl_rw_derr
• flash_ctrl_sec_cm
• flash_ctrl_write_word_sweep
• flash_ctrl_rw_serr
• flash_ctrl_prog_reset
• flash_ctrl_rw_evict
• flash_ctrl_derr_detect
• flash_ctrl_serr_address
• flash_ctrl_serr_counter
• flash_ctrl_rw_evict_all_en
• flash_ctrl_fs_sup

@matutem: I think you landed a large flash_ctrl PR a few days ago. Would you mind taking a look at some of these: I think your PR might well be responsible for the failures.

Steps to Reproduce

• GitHub Revision: ae68723

Tests with similar or related failures

No response

[vogelpi]

Those failures are all due to enabling ECC more widely in block-level DV some time ago. Before doing this, the nightlies all looked good. The last RTL change was already before that.

In the meantime, @matutem has been adjusting more block-level DV sequences to let them deal with ECC errors correctly. There are still some to do but I am not to worried about this.

[johngt]

@vogelpi to update test list

[vogelpi]

The current status (May 19 nightly regression) is as follows:

• 100 % pass rate for all tests except for
• flash_ctrl_rw_evict - 70%
• flash_ctrl_rw_evict_all_en - 85%
• flash_ctrl_oversize_error - 80%
• flash_ctrl_intr_rd - 95%
• flash_ctrl_intr_wr - 90%
• flash_ctrl_read_word_sweep_derr - 40%
• flash_ctrl_rw_derr - 60%
• flash_ctrl_integrity - 40%
• flash_ctrl_read_word_sweep_serr - 20%
• flash_ctrl_rw_serr - 80%
• flash_ctrl_wo - 85%
• flash_ctrl_rw - 95%
• flash_ctrl_fs_sup - 80%
• flash_ctrl_config_regwen - 80%

[andreaskurth]

Discussed in triage: moving to M5 as DV issue because RTL hasn't been changed in a while, many tests passed before enabling ECC in DV, and the remaining DV fails seem to be caused by DV problems rather than RTL problems

[moidx]

@matutem is still pending regression results for the following test cases:

1. flash_ctrl_read_word_sweep_derr
2. flash_ctrl_integrity

[andreaskurth]

Moving this issue to M7, as just discussed in triage meeting. Those tests fail because ECC checks have recently been enabled, but flash_ctrl was already signed off at V2S without those checks. It is out of scope of M5 to bring flash_ctrl DV to a state in which all tests pass for more than 90% of the seeds with ECC checks enabled.

We think the risk for critical RTL bugs in flash_ctrl is lower than when it was signed off at V2S because DV now covers the ECC feature and most tests pass. Also, flash_ctrl is getting exercised in almost all top-level tests as well as in almost all SiVal tests, and no critical bugs have been found.

[vogelpi]

Based on last week's discussion, I decided it was useful to document the history of this and the current state in terms of regression failures and how all this aligns with the V2S signoff, as well as with the RTL and DV changes done for Earlgrey-PROD.

Below, you can see an overview of the regressions results of the last couple of months. Every column corresponds to one regression run, every row corresponds to one test (note that I had to split the many tests into four views, i.e., the stacked images). I've annotated the most important RTL and DV PRs to corresponding regression runs.

What we can see:

• When the V2S signoff got prepared, everything looked green.
• The RTL change to share the scrambling module (required to save area for M2), resulted in quite some test failures.
• The RTL change to fix the scrambling arbitration request logic fixed all these regression failures.
• The RTL change allowing firmware to deal with multi-bit ECC and ICV errors (required for integration) had no impact on the regression.
• The RTL change to pop the data & mask FIFOs in sync helped fixing a regresssion (second view) but did not impact other tests.
• The DV change to enable random read write tests with ECC enabled had a much higher impact than anticipated. We recognized this and opened this very issue. @matutem worked very hard to close out these test failures which resulted in the many DV PRs, here I am just mentioning the most relevant ones:
  • Fix failures with ECC enabled - improved the regression again massively
  • Fix some test failures - again had a notable impact on the regression
• The RTL changes to improve the security hardening prior to RTL freeze ([flash_ctrl/rtl] Enhance data intg in flash_phy_rd #23514, [flash_ctrl/rtl] Enable SecFifoPtr for TL-UL FIFOs #23515, [flash_ctrl/rtl] Host data XOR address infection #23557) had no impact on the nightly regressions.
• The DV changes ([dv,flash_ctrl] Avoid double writes in flash_ctrl_rw_evict_vseq #23682, [dv,flash_ctrl] Initialize data for flash_ctrl_read_word_sweep #23726) further improved the nightly regressions.
• The last RTL change to fix the read buffer re-allocation performance issue also did not impact the nightly regresssions.
• The latest DV change to fix some more block-level failures fixed some failures but also seems to have caused some new ones (flash_ctrl_derr_detect).

So to summarize:

• The only RTL change which had a negative impact on the regression was the sharing of the scrambling module to save area before M2.
• Follow-up fixes were done to successfully improve the regression results again.
• Switching on ECC in more tests impacted the regression results massively. Cleaning these test failures up is a major effort which is still ongoing. Thanks to the great work of @matutem we do now have a better understanding of the limitations of flash_ctrl DV and we have higher confidence in the design and DV.

Why ECC was switched off in so many tests is unclear and wasn't documented. We doubt that there was a good reason to disable this core feature in so many tests. Ideally, we would clean up all these failures of course but we shouldn't gate on this. Overall, I am convinced we are in a better shape on all sides compared to ES (DV, RTL, security) and I believe we shouldn't worry about Flash regarding Earlgrey-PROD.

FYI @andreaskurth , @matutem , @johngt, @moidx , @jonmichelson

[johngt]

Thanks for the summary @vogelpi

[hayleynewton]

FYI, Look at the flash_ctrl tests based off the recent results.

Tests included in this ticket which fall below 100%:

80:
Flash_ctrl_serr_counter
Flash_ctrl_oversize_error
Flash_ctrl_rw_evict
Flash_ctrl_rw_derr
Flash_ctrl_rw_serr

0%:
flash_ctrl_derr_detect

We also have some outside of the ones flagged here:

Tests which fall below 100%:

80:
Flash_ctrl_ro
Flash_ctrl_rw
Flash_ctrl_phy_ack_consistency