lowRISC / opentitan

OpenTitan: Open source silicon root of trust
https://www.opentitan.org
Apache License 2.0
2.58k stars 776 forks source link

[dv/csr] Handle reset in the middle of CSR transactions #15085

Open matutem opened 2 years ago

matutem commented 2 years ago

The DV code dealing with CSR accesses gets confused if a reset happens while there is an ongoing CSR access. In order to avoid this confusion tests are restricted to avoid resets while there are ongoing CSR accesses. This is undesirable on two accounts:

matutem commented 2 years ago

Correction: the scenario is actually generated, but the problem is that the sequence cannot terminate in the middle of a transaction. Perhaps csr_rd/wr should immediately return if cst_utils_pkg::under_reset is on to avoid sequences getting wedged?

In any case it would be smart to document this behavior to avoid time wastage.

tjaychen commented 2 years ago

@weicaiyang @sriyerg

might need your help to gauge whether this is something that's M2 gating or not. It sounds like to me it's closer to V3 / M3 ..

weicaiyang commented 2 years ago

I have discussed this offline with @matutem. We don't restrict a sequence to issue a reset in the middle of a CSR access, but RAL access can't be killed before it finishes, otherwise, RAL is locked up. We could either wait until all CSR accesses are done, before killing the sequence, or use the stress_all_with_rand_reset approach. @matutem LMK if you have any problem on this issue. I think it's ok to close this or defer to V3. stress_all_with_rand_reset is a V3 test.

tjaychen commented 2 years ago

personally i think killing after completion is probably better... but sounds good, let's table this to M3 for now.

If anyone objects, please add to the thread.

mundaym commented 1 year ago

Triage: not considering this for M2.5 currently.

andreaskurth commented 1 year ago

Triage: not considering this for M2.5 currently.

Rationale: With the current reset architecture, we have ndmreset and POR, which both reset blocks with CSRs as well as Ibex. There are also SW-controlled resets (e.g., for spi_device), but when Ibex does a reset through that, it cannot simultaneously do a CSR access during the reset.

matutem commented 1 year ago

Should this be reassigned?

andreaskurth commented 1 year ago

Should this be reassigned?

Yeah, it's currently scheduled for M3, so I think we'll assign it when focusing on M3