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

Fixing the address-wrapping RISC-V compatibility issue #111

Open
tariqkurd-repo opened this issue Aug 24, 2023 · 5 comments
Open

Fixing the address-wrapping RISC-V compatibility issue #111

tariqkurd-repo opened this issue Aug 24, 2023 · 5 comments

Comments

@tariqkurd-repo
Copy link

tariqkurd-repo commented Aug 24, 2023

A quick thought on removing the fact that CHERI can't allow the case (almost certainly caused by a software bug) where a misaligned load/store is allowed to wrap the address space on a RISC-V but disallowed on a CHERI core.

If we define almighty_cap differently - so that instead of it setting maximum bounds covering the full 64-bit address space - we say that the specific programming means "bounds checks disabled" then it will work out nicely.

It's introducing a special case, but it also solves a tricky problem.

It also means that almighty can be defined differently, it could be a permission / status bit or a specific out of bounds exponent value, not necessarily encoded in the bounds in the conventional sense.

@andresag01
Copy link

The CHERI specification is already set up to accept address wrapping when checking bounds against the almighty capability. However, it looks like the reference SAIL does not implement this properly in some cases (e.g. when checking data accesses); there might also be bugs around this area in other CHERI implementations.

@tariqkurd-repo tariqkurd-repo changed the title Fixing the address-wrapping RISC-V compatiblity issue Fixing the address-wrapping RISC-V compatibility issue Sep 4, 2023
@jonwoodruff
Copy link
Contributor

Maybe we only need a small change to the SAIL to not throw any bounds exception when the Exp==MaxExp?

@jrtc27
Copy link
Member

jrtc27 commented Sep 5, 2023

That's a way to implement it, but perhaps not the "nice" way to have it specified

@tariqkurd-repo
Copy link
Author

tariqkurd-repo commented Nov 20, 2023

Variable XLEN makes the situation more complex here.
32-bit user mode (integer mode only) with 64-bit supervisor mode (in capability mode).

In this case the user mode accesses are checked by the wider capabilities defined by supervisor mode.

Detecting 32-bit wrapping against a RV32 capability is one problem to solve, but to compare against a wider capability which can encompass the top or bottom or both of the 32-bit space, and take the wrapping into account, is more complex.

My proposal for all of these cases is to introduce a misaligned exception for wrapping RV32 load/store accesses. CHERI already adds a lot of exceptions to existing RISC-V code, and misaligned exceptions are a standard architectural feature.

It is already permitted for hardware which doesn't support misaligned accesses to take an exception and ask the OS to fix up the result in software, so the memory wrapping case becomes a corner case of that.

In real systems this won't happen, so hiding it behind an exception makes life a lot simpler.

@jrtc27
Copy link
Member

jrtc27 commented Nov 21, 2023

No, we should just make the architecture work. Introducing a new exception breaks compatibility, is extremely ugly and does not work well when you have things like vector loads and stores that can legitimately be less aligned than their size (e.g. the whole register loads and stores). I really do not think you want to be having to emulate every single load and store in M-mode, complete with every single CHERI check, at risk of screwing up and breaking the architecture. Better to just specify the edge cases that need careful handling, have some tests for them and move on rather than add piles of software to make hardware's life slightly easier.

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

No branches or pull requests

4 participants