-
Notifications
You must be signed in to change notification settings - Fork 877
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
triggers: Let mcontrol.match be default (0/equal) if maskmax is 0 #1786
Conversation
@@ -294,13 +294,14 @@ void mcontrol6_t::tdata1_write(processor_t * const proc, const reg_t val, const | |||
auto xlen = proc->get_const_xlen(); | |||
assert(get_field(val, CSR_MCONTROL6_TYPE(xlen)) == CSR_TDATA1_TYPE_MCONTROL6); | |||
dmode = get_field(val, CSR_MCONTROL6_DMODE(xlen)); | |||
const reg_t maskmax6 = xlen - 1; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rtwfroody I didn't find an explicit upper bound for maskmax6 in the Debug spec. Does the spec intend maskmax6=31
in RV32 and maskmax6=63
in RV64 for maximum?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. It doesn't make sense for maskmax6 to be any larger than that, because then you're just doing an equal comparison.
case MATCH_NAPOT: if (maskmax == 0) return MATCH_EQUAL; | ||
case MATCH_EQUAL: | ||
case MATCH_NAPOT: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems tricky. Invite comment on this modification.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks OK to me.
To be clear, it explicitly removes napot support from mcontrol, and makes it work for mcontrol6.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code looks OK; I didn't review the functionality in detail.
This PR fixes the issue in #1785.
Specifically, the spec requires unsupporting
mcontrol.match=NAPOT
ifmcontrol.maskmax=0
.