-
Notifications
You must be signed in to change notification settings - Fork 9
src/mte_tag: HLV/HSV support for opt-out based on PTE.MTAG for codepage #77
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
base: main
Are you sure you want to change the base?
Conversation
|
HLV/HSV operate with guest-virtual addresses, and behave as if performing a load/store in V=1, including two-stage address translation, which picks the same PTE.MTAG as the guest would. |
MTAG bit is used for code page and data page differently. Specification says that setting MTAG for a page with RWX is PTE illegal encoding Now coming back to this PR. |
|
Thanks, I forgot the instruction's page matters. Yeah, that is a gap, where the hypervisor would have to walk guest's SATP, which is what HLV/HSV want to prevent... I am not overly happy with the design, as we're making the instruction information provided by htinst incomplete, and hypervisor now has to issue hlvx where it would previously just read htinst. |
We can spill that information in hstatus too. Let me revise it. And yes I do share the overall ickiness you are having. However, software |
I'll close this PR because I believe it's a bit too much overhead on higher priv isa. Instead I'll come with new PR with following
Let me know what you think on this approach. |
1f112d1 to
48845a1
Compare
I take it back :-). I did try that route and after spending a bit of time, it was another rabbit hole. I have revised this PR to make it a little bit more manageable. Please take a look. |
48845a1 to
3a1f73d
Compare
Seems like we cornered ourselves with the elision based on instruction's PTE... Was it that important? :) The patch makes sense given the current constraints. |
a386aaa to
63d480f
Compare
src/mte_tag.adoc
Outdated
|
|
||
| hstatus.MTAG_I = VS-stage_leaf_PTE(s).MTAG & VS-stage_leaf_PTE(s).X | ||
|
|
||
| If `vsat.MODE == BARE`, then `hstatus.MTAG_I` will always be clear. |
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.
typo: vsatp
src/mte_tag.adoc
Outdated
|
|
||
| mstatus.MTAG_I = first-stage_leaf_PTE(s).MTAG & first-stage_leaf_PTE(s).X | ||
|
|
||
| If `sat.MODE == BARE`, then `mstatus.MTAG_I` will always be clear. |
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.
typo: satp
src/mte_tag.adoc
Outdated
| [[MPRV_LDST]] | ||
| === Memory tagging on loads/stores affected by Modify Privilege bit (MPRV) | ||
|
|
||
| If execution environment is M-mode, MPRV and MXR both are set, and memory |
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.
The MXR bit can also be enabled by S-mode, possibly to read U-mode execute-only pages.
Should mstatus.MTAG_I (also visible in sstatus) bit be set when accessing any virtual memory with MXR?
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.
Even if SUM=1, effective privilege is still PRV=S.
Supervisor accesses to user pages don't result in tag checks. There is no equivalent MPRV for S.
I expect MXR usage in S to primarily read code page which was execute-only.
We can drop M-mode execution environment requirement above and simply say
"""
If MPRV and MXR both are set ....
"""
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.
Right, the whole architecture falls apart when implementing U-mode memory accesses in S-mode.
Do we instead recommend that S-mode software should execute a short snippet in U-mode, e.g. ld a0, a1; ecall, to utilize the tagging hardware?
Alternative recommendation could be to use the H-extension's HLV/HSV/HLVX with hgatp=0 to perform the U-mode memory accesses.
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.
That can be done but not with hgatp = 0.
HLV/HSV expect to use vstap and hgatp, do a two-stage page walk.
If someone really wants to do that, they would have to reflect satp in vsatp. And then ensure that entire physical memory is identity mapped via hgatp.
right?
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 hgatp=0 is an identity mapping in the two-stage page walk, but doing a full identity mapping with single level hgatp isn't that much more complicated either.
vsatp=satp, hstatus.SPVP=0, possibly few other controls, like endinaness, and likely a tlb flush should prepare the environment.
src/mte_tag.adoc
Outdated
|
|
||
| mstatus.MTAG_I = first-stage_leaf_PTE(s).MTAG & first-stage_leaf_PTE(s).X | ||
|
|
||
| If `sat.MODE == BARE`, then `mstatus.MTAG_I` will always be clear. |
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.
mstatus.MTAG_I should be collected as 0 when satp.MODE == Bare, but the current wording requires the implementation to prevent software from setting mstatus.MTAG_I to 1, based on satp.MODE, which doesn't seem necessary.
Can we soften the wording into something like: when satp.MODE == Bare, the pte.MTAG and pte.X bits are considered to be 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.
Is below ok?
"""
If satp.MODE == BARE, then mstatus.MTAG_I is not collected.
"""
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 mstatus.MTAG_I still needs to be collected, even though we know it will be set to 0, since it may have a value from a previous S-mode context where satp.MODE != BARE. So I would suggest something like:
If
satp.MODE == BARE, then the collected value ofmstatus.MTAG_Iwill be 0.
|
Can anyone take a look, approve or drop a comment? |
src/mte_tag.adoc
Outdated
| === Memory tagging on loads/stores affected by Modify Privilege bit (MPRV) | ||
|
|
||
| If execution environment is M-mode, MPRV and MXR both are set, and memory | ||
| tagging is enabled for effective privilege then load instruction collect MTAG |
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 is inconsistent with the wording for HLVX above, which overwrites hstatus.MTAG_I regardless of if memory tagging is enabled for the effective privilege level.
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.
Good catch. thanks. will fix it.
src/mte_tag.adoc
Outdated
|
|
||
| mstatus.MTAG_I = first-stage_leaf_PTE(s).MTAG & first-stage_leaf_PTE(s).X | ||
|
|
||
| If `sat.MODE == BARE`, then `mstatus.MTAG_I` will always be clear. |
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 mstatus.MTAG_I still needs to be collected, even though we know it will be set to 0, since it may have a value from a previous S-mode context where satp.MODE != BARE. So I would suggest something like:
If
satp.MODE == BARE, then the collected value ofmstatus.MTAG_Iwill be 0.
src/mte_tag.adoc
Outdated
| privilege mode is VU. `hstatus.MTAG_I` bit emulates `MTAG` bit for | ||
| instruction fetch from code page. When a trap is taken to HS mode and `htinst` | ||
| is written with nonzero value, then VS-stage page table's MTAG bit on code page | ||
| for qualifying instruction is deposited in `hstatus.MTAG_I`. Furthermore, when |
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.
What is a "qualifying instruction"? I would expect hstatus.MTAG_I to be set regardless of the instruction For example, the hypervisor could emulate a load/store instruction that the hardware doesn't know about, so hstatus.MTAG_I needs to be set for the Illegal Instruction exception, even though the hardware doesn't consider this to be a load/store.
src/mte_tag.adoc
Outdated
| for qualifying instruction is deposited in `hstatus.MTAG_I`. Furthermore, when | ||
| HLVX* instructions walks VS-stage page table to fetch instruction then | ||
| `hstatus.MTAG_I` is set to bitwise AND of VS-stage leaf entry's `MTAG` and | ||
| execute permission bits. |
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 would recommend referencing the earlier paragraph instead of partially reproducing the logic here.
824c0dc to
69a8ee2
Compare
|
@radimkrcmar , @SiFiveHolland |
…odepage Guest might have set PTE.MTAG for code page and thus wouldn't be expecting tag checks even if data pointer had PTE.MTAG set. On an exit to hypervisor, hypervisor emulating this load/store must do emulation similarly and thus HLVX* is updated to collect to MTAG bit in hstatus. MTAG_I and use that during HLV/HSV. Similar mechanisms when in M-mode and MPRV=1. Signed-off-by: Deepak Gupta <[email protected]>
69a8ee2 to
52aac85
Compare
|
|
||
| The Zimt extension adds `MTAG_I` bit to `mstatus`. When a trap is taken to | ||
| M-mode and `mtval` is written with nonzero value, then MTAG bit for code page | ||
| of trapping instruction is deposited in `mstatus.MTAG_I`. |
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.
We're basing MTAG_I on htinst for traps to HS-mode, but a trap to M-mode depends on mtval instead.
Should we make M-mode depend on mtinst as well?
M-mode does have instruction in mtval for illegal-instruction, does that change what is written in mtinst, so we ought to depend on any CSR where an instruction is written?
Guest might have set PTE.MTAG for code page and thus wouldn't be expecting tag checks even if data pointer had PTE.MTAG set. On an exit to hypervisor, hypervisor emulating this load/store must do emulation similarly and thus HLVX* is updated to collect to MTAG bit in hstatus. VMTAG_FETCH and use that during HLV/HSV.