Skip to content
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

CLoadTags: Try accessing the first and last capability-size chunk #233

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

arichardson
Copy link
Member

Check that we can access the first and last capability-size chunk. This also ensures that the bad access address is updated correctly in case any of these addresses are not accessible. In theory we should try to access the entire memory region (to match what sail does), but first and last will be equivalent for trapping/RVFI.

Previously we were just checking the tag memory, which returns 0 for unmapped memory regions, but we should trap if these accesses are prevent e.g. by the PMP.

TestRIG-generated and annotated trace:

.4byte 0x0108185b # cincoffsetimmediate x16, x16, 16
.4byte 0xfb08185b # cincoffsetimmediate x16, x16, 4016
.assert rd_wdata == 0xffffffffffffffc0 "expected addr"
.4byte 0xff2808db # cloadtags x17, x16
.assert trap == 1 "Bad addr"
.assert mem_addr == 0xffffffffffffffc0 "Should be initial aligned address not last address"
.4byte 0x342020f3 # csrrs x1, mcause (0x342), x0
.assert rd_wdata == 5 "RISCV_EXCP_LOAD_ACCESS_FAULT"
.4byte 0x343020f3 # csrrs x1, mtval (0x343), x0
.assert rd_wdata == 0xffffffffffffffc0 "Tval should be bad addr"
```

@arichardson arichardson requested review from nwf and jrtc27 August 3, 2023 17:09
@jrtc27
Copy link
Member

jrtc27 commented Aug 3, 2023

Won't this do wonky things if this is I/O memory? Not sure what hardware does in those cases though... but IMO that should be well-defined, either as a load access fault or as always-zero. Given LC of I/O memory gives a tag of 0 I feel it should be the latter.

@jrtc27 jrtc27 requested a review from PeterRugg August 3, 2023 17:17
Check that we can access the first and last capability-size chunk.
This also ensures that the bad access address is updated correctly
in case any of these addresses are not accessible.
In theory we should try to access the entire memory region (to match what
sail does), but first and last will be equivalent for trapping/RVFI.

Previously we were just checking the tag memory, which returns 0 for
unmapped memory regions, but we should trap if these accesses are prevent
e.g. by the PMP.
@arichardson arichardson force-pushed the cloadtags-bad-access-address branch from 966f62a to 7a2fee0 Compare August 3, 2023 17:20
@arichardson
Copy link
Member Author

Won't this do wonky things if this is I/O memory? Not sure what hardware does in those cases though... but IMO that should be well-defined, either as a load access fault or as always-zero. Given LC of I/O memory gives a tag of 0 I feel it should be the latter.

As far as I can see sail does capability loads and therefore has the exact same behaviour as a capability load. This should match sail - cap_tag_load_many deals with the capability MMU bits and the data load does the remaining PMP,I/O,etc. checks.

@PeterRugg
Copy link

I'm not actually certain what Toooba will give if you try to access I/O memory: if the line isn't in cache (which it won't be) it should go up to the tag controller, which won't know what to do with it. That will depend on what Hongyan did, which I can check.
Regardless, what it should do: I agree it should either be hardwired to zero or be defined to match the lc, so throw a trap if required. If the address isn't mapped, you should get a VM exception exactly the same as on a capability load. Since it's supposed to happen per-cacheline, we would ideally not have to worry in the spec about some tags being mapped but not others. Whether you're allowed to do the naive load and trigger read side-effects is yet another question: currently Flute will, because it just loads the cap, and Toooba won't, because it asks the tag controller.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants