-
Notifications
You must be signed in to change notification settings - Fork 1k
Add Svukte extension support #2121
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
Conversation
aswaterman
left a comment
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.
@ingallsj can I delegate to you to review this, particularly the various cases in check_svukte_qualified?
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.
Hi @aswaterman
Modification by your suggestion has been done.
Many thanks for review
binno
left a comment
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.
Hi @aswaterman
Modification by your suggestion has been done.
Please help to review it again
This adds support for the Svukte extension, which adds support for address-independent latency of user-mode faults to supervisor addresses.
| if (proc->extension_enabled('S') && get_field(state->senvcfg->read(), SENVCFG_UKTE)) { | ||
| if (forced_virt && state->prv == PRV_U) { | ||
| bool hstatus_hukte = proc->extension_enabled('H') && (get_field(state->hstatus->read(), HSTATUS_HUKTE) == 1); | ||
| return !hstatus_hukte; |
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.
I feel like there should be an address check before this return statement.
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.
I think there's both an actual bug and a conceptual bug here. Per the spec, "Svukte-qualified" is a property of an access type, not an address. Either this function should not accept an address arg, or it should be renamed. That might be why this bug cropped up. I'll make a fix.
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.
I think the address check is in line 587
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.
But if you hit the return statement, you never reach like 587, so the address never gets checked for HLV/HSV in U-mode.
I think my PR #2171 fixes this case.
|
|
||
| assert(proc->get_xlen() == 64); | ||
| if ((addr >> 63) & 1) { | ||
| return (state->v || forced_virt) && ((state->vsatp->read() & SATP64_MODE) == 0); |
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.
Am I understanding this return statement correctly, that we pass the Svukte check if in VU mode and vsatp.MODE == Bare? Where do we check satp for when in HU mode?
This adds support for the Svukte extension, which adds support for address-independent latency of user-mode faults to supervisor addresses.