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

CHERI specification for RISC-V Cache Management Extension #65

Open
andresag01 opened this issue Jul 14, 2023 · 11 comments
Open

CHERI specification for RISC-V Cache Management Extension #65

andresag01 opened this issue Jul 14, 2023 · 11 comments

Comments

@andresag01
Copy link

RISC-V has a cache management extension with instructions for operations like flush, clean, prefetch, etc. It would be good to write the specification for these instructions in the CHERI-RISC-V context. The bounds, access, etc checks for these instructions are different when compared to load/store instructions.

Are there currently any CHERI proposals in connection with the RISC-V cache management extension?

@bsdjhb
Copy link
Collaborator

bsdjhb commented Jul 14, 2023

Nothing is written, yet, no. In part I think this would be part of a larger effort to evaluate more recent changes to the privileged spec. There are some important lessons to be learned here from Morello which requires bounds to cover the entire cache line which is not ideal. Probably you just want the bounds to overlap with at least part of the region you are flushing (at least for flush and prefetch) as the actual write was already validated and flush is just changing the "timing" of a commit, and similarly for prefetch the actual read will be validated in the future.

@jrtc27
Copy link
Member

jrtc27 commented Jul 14, 2023

Invalidate is the interesting one. RISC-V made the, in my opinion, dangerous and misguided choice to not require write permission in the corresponding PTE (contrast with Arm, which does). For invalidate the only sensible option is to require bounds of the whole line and write permission, since otherwise it is a hole through which you can revert writes.

@arichardson
Copy link
Member

Invalidate is incredibly dangerous since it could be used to restore capabilities that were expected to be deleted. IMO the only sane thing is to not allow invalidates or require an almighty capability.

@andresag01
Copy link
Author

andresag01 commented Jul 17, 2023

As far as I know, the RISC-V CMO spec allows disabling invalidations in lower privileged levels by writing bits in a CSR. It is possible to either cause a trap when an invalidation is performed or simply perform a cbo.flush in place of an invalidation depending on the CSR configuration. I do not know if it a compliant implementation can completely get rid of the cbo.inval instruction.

@tariqkurd-repo
Copy link

tariqkurd-repo commented Jul 17, 2023

it's valid to do that - I asked for clarification earlier this year - so I think what we should do is strongly recommend that when CHERI is implemented that CBO.INVAL always operates as CBO.FLUSH. Apart from anything else, it's always valid to use CBO.FLUSH as you never know whether a line is in the cache or not.

@tariqkurd-repo
Copy link

To answer @bsdjhb 's comment from above - we need to write some rules for this (which accidentally turned into a new issue when I try to reply to his comment)

CBO.ZERO must clearly cover the whole cache line, as every byte is written.
CBO.INVAL -> remap to CBO.FLUSH
CBO.FLUSH -> bounds cover at least part of the cache line? Must it include the VA?
PREFETCH.* -> do we need bounds checking? Are we introducing a side-channel if we don't include any? As you say the actual read is accurately validated, so we're only issuing a hint to update the cache state.

@andresag01
Copy link
Author

After discussions, the current proposal is as follows:

  • prefetch.i, prefetch.r, prefetch.w, cbo.flush and cbo.clean operate exactly as described in the RISC-V specification. These instructions remain unaffected by the PCC mode flag.
    • These instructions do not provide data/instruction access and make no architecturally visible changes, so it is safe to not require capabilities to execute them.
  • cbo.zero is a whole cache line store, so it behaves and is checked exactly like any other RISC-V store
    • Bounds, tag, store permission, etc are checked
    • There are two variants of the instruction depending on the PCC mode flag
  • cbo.inval: This instruction is complicated because its behavior is kind of "undefined" in the sense that there are no guarantees regarding the value that ultimately remains in memory for the cache line that is invalidated. The safest option is to perform it as a cbo.flush, but a CHERI-safe alternative is:
    1. Load, store, load cap, store cap and ASR permissions
    2. The entire cache line must be in-bounds
    3. The data itself may be invalidated from the cache, but the cache line's capability tags must be zeroed and written back
    4. The cbo.inval must retain checks (i) and (ii) even if it behaves like cbo.flush
    5. There are two versions of this instruction depending on the PCC mode flag

@jrtc27
Copy link
Member

jrtc27 commented Sep 8, 2023

The data itself may be invalidated from the cache, but the cache line's capability tags must be zeroed and written back

This is very odd. I get that it’s a dangerous instruction, but this is not an invalidate operation, an invalidate is to drop everything and read what’s in DRAM again, which would include whatever tags DRAM has.

@jrtc27
Copy link
Member

jrtc27 commented Sep 8, 2023

These instructions do not provide data/instruction access and make no architecturally visible changes, so it is safe to not require capabilities to execute them.

I don’t see a good reason not to, and code that wants to talk about memory it doesn’t have access to is dodgy anyway. Prefetching should silently become a nop given an invalid capability, and flush/clean likely should fault. Otherwise it sounds like you’re potentially giving people useful gadgets for speculation attacks. This could be of significant concern in a compartmentalised environment. I would be uncomfortable not having these checks.

@tariqkurd-repo
Copy link

The data itself may be invalidated from the cache, but the cache line's capability tags must be zeroed and written back

This is very odd. I get that it’s a dangerous instruction, but this is not an invalidate operation, an invalidate is to drop everything and read what’s in DRAM again, which would include whatever tags DRAM has.

I guess that if you really want an invalidate then you shoudl get just that. The concern is that rogue invalidations could reveal stale capabilities which have been overwritten.

Maybe you need controls on it per privilege level (or ring), and / or an invalidate permission bit?

@andresag01
Copy link
Author

As discussed:

  • cbo.zero is a whole cache line store, so it behaves and is checked exactly like any other RISC-V store
    • Bounds, tag, store permission, etc are checked
    • There are two variants of the instruction depending on the PCC mode flag
  • prefetch.i, prefetch.r and prefetch.w
    • Tag and sealing check as usual
    • prefetch.i check execute permission
    • prefetch.w check store permission
    • prefetch.r check load permission
    • Check that the authorising capability's bounds cover at least 1 byte in the cache line prefetched
    • Do not start the prefetch if any of the checks above fails.
  • cbo.clean and cbo.flush
    • Tag and sealing check as usual
    • Check that there are store or load permissions
    • Check that the authorising capability's bounds cover at least 1 byte in the cache line flushed/cleaned
  • cbo.inval
    • Tag and sealing check as usual
    • Check that there are store, load and ASR permissions
    • Check that the authorising capability's bounds cover the entire cache line invalidated
    • The same checks must be retained if cbo.inval is configured to behave as cbo.flush

sorear added a commit to sorear/riscv-cheri that referenced this issue Jan 26, 2024
The exception table for CMOs comes from CTSRD-CHERI/cheri-specification#65.
sorear added a commit to sorear/riscv-cheri that referenced this issue Jan 26, 2024
The exception table for CMOs comes from CTSRD-CHERI/cheri-specification#65.
sorear added a commit to sorear/riscv-cheri that referenced this issue Feb 2, 2024
The exception table for CMOs comes from CTSRD-CHERI/cheri-specification#65.
sorear added a commit to sorear/riscv-cheri that referenced this issue Feb 2, 2024
The exception table for CMOs comes from CTSRD-CHERI/cheri-specification#65.
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

5 participants