Fix for #481 #483
Fix for #481 #483
Conversation
Signed-off-by: Arjan Bink <[email protected]>
|
Hi @silabs-PaulZ , @silabs-oysteink This pull request should fix #481 Some of the reserved 0 bits were writeable as well and I made them always-read-as-0 instead. I would like to hear your opinion on 'dcsr_n.prv = priv_lvl_q; '; I think this bit should be fully R/W but as the spec says "If the encoding written is not supported or the debugger is not allowed to change to it, the hart may change to any supported privilege level." (So I think we should change that assignment into 'dcsr_n.prv = csr_wdata_int[1:0]; ' and then always simply ignore the written value (and basically pretend it is alway M mode). |
|
I agree with your suggestion Arjan. The only side effect I can think of right now is if it affects the Imperas reference model, but if it does it's probably a quick fix there as well. |
Signed-off-by: Arjan Bink <[email protected]>
|
I was starting the process to update stepie to be read/writeable, but it would be fairly involved. I agree with tying it stepie=0. The implementation looks correct for tying this off (along with other WARL fields). |
rtl/cv32e40p_cs_registers.sv
Outdated
| priv_lvl_n = dcsr_q.prv; | ||
| // Restore to the recorded privilege level; if dcsr_q.prv is a non-supported mode, | ||
| // then lowest privilege supported mode is selected (so always Machine Mode in this case). | ||
| priv_lvl_n = (dcsr_q.prv == PRIV_LVL_M) ? PRIV_LVL_M : PRIV_LVL_M; |
There was a problem hiding this comment.
I would consider priv_lvl_n = PRIV_LVL_M in this case for any code coverage issues. Hopefully we can refactor this file soon to reduce the duplication issues.
There was a problem hiding this comment.
Hopefully we can refactor this file soon to reduce the duplication issues.
Its getting late in the day for RTL refactoring exercises. If you are serious about this, please open an issue so the cost/risk/benefit can be discussed.
There was a problem hiding this comment.
I would consider priv_lvl_n = PRIV_LVL_M in this case for any code coverage issues. Hopefully we can refactor this file soon to reduce the duplication issues.
Okay, I will rewrite like that and merge this pull request. Thanks.
Signed-off-by: Arjan Bink <[email protected]>
Signed-off-by: Arjan Bink [email protected]