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

Split CRG functionality for U and S-modes #491

Merged
merged 13 commits into from
Jan 15, 2025

Conversation

tariqkurd-repo
Copy link
Collaborator

@tariqkurd-repo tariqkurd-repo commented Dec 17, 2024

@jonwoodruff
Copy link
Contributor

To provide a bit of context, this was prompted by questions from @buxtonpaul about how to use the PTE bits, which involved setting up a null configuration for kernel-privileged pages. This then made obvious that the kernel pages were guarded by the same generation as user pages, and so sstatus would need to be swapped on kernel entry, which we believe is not the case currently. A better solution is to have a dedicated bit for the supervisor generation.

@jrtc27
Copy link
Collaborator

jrtc27 commented Dec 17, 2024

I misinterpreted the linked issue as being about kernel revocation and adding extra PTE bits, not about ensuring the kernel can actually load from kernel pages regardless of userspace's generation for userspace pages. Do we think that having sstatus.SCRG would form part of the story for kernel revocation? If there's a risk it's not what we'd want then it might be better to leave the sstatus bit (and PTE bit) reserved and say that there are no generation checks for S-mode pages. But probably it's something we'll need...

src/cheri-pte-ext.adoc Outdated Show resolved Hide resolved
src/cheri-pte-ext.adoc Outdated Show resolved Hide resolved
@nwf
Copy link
Collaborator

nwf commented Dec 17, 2024

@jrtc27 Has the right of it: kernel access to user memory should generally be gated, for revocation purposes, as if it were performed by the user program.

The existing implementation performs its page sweeps by bypassing the PTE bits and their load barrier effect by using the "direct map" region of the address space, manually translating virtual addresses to (offset) physical ones. These accesses would look, to hardware, like the kernel accessing kernel mappings.

There is a proposal for a privileged "as user capability load, bypassing the CRG fault" instruction that would let us simplify much of that logic down to allowing pages to be swept directly in virtual address space (and possibly even concurrently with address space manipulation, currently emphatically prohibited due to the direct access to physical memory involved), but that's still a lot of engineering and measurement to make sure it's a good idea to be done.

All of that to say, I'm not sure a "CRGS" bit is the right idea. Until someone cracks kernel sweeping revocation, a much more challenging problem than for userspace memory, the kernel's own access to virtual memory will be to pages that are almost surely parked in some capability-permissive but CRG-less PTE configuration, assuming we have one of those. If we don't, it's probably simpler to just not raise CRG faults on PTEs with their U bit clear?

@tariqkurd-repo tariqkurd-repo changed the title Split sstatus.CRG into CRGS/CRGU Split sstatus.CRG into SCRG/UCRG Dec 18, 2024
@jonwoodruff
Copy link
Contributor

jonwoodruff commented Dec 18, 2024

I'm certainly on-board with the far simpler, "non-user privileged pages don't ever get generation traps." With apologies to @tariqkurd-repo for dragging him through the mud, if we did agree on that one, would it be better to modify this pull request into that change to keep the history and discussion? Or start a new one? (Such a modification would basically be reverting most/all of it and adding a note somewhere. I guess.)

@tariqkurd-repo tariqkurd-repo changed the title Split sstatus.CRG into SCRG/UCRG SPlit CRG funcitonality for U and S-modes Jan 9, 2025
@tariqkurd-repo tariqkurd-repo changed the title SPlit CRG funcitonality for U and S-modes Split CRG functionality for U and S-modes Jan 10, 2025
@tariqkurd-repo
Copy link
Collaborator Author

tariqkurd-repo commented Jan 10, 2025

Two big changes here in the last commit:

  1. remove sstatus.SCRG, and leave kernel space revocation for future
  2. make PTE.CW mandatory, so the default is to not allow tagged caps to be stored for legacy OSs

and so Zcheripte includes CRG but not CW

Overall, this PR:

  1. renames sstatus.CRG to sstatus.UCRG and makes it only tested in user mode
  2. has no support for kernel space revocation (sstatus.SCRG is not specified)
  3. makes PTE.CW mandatory and PTE.CRG+status.UCRG optional but strongly recommended

@tariqkurd-repo tariqkurd-repo requested a review from jrtc27 January 10, 2025 11:20
src/cheri-pte-ext.adoc Outdated Show resolved Hide resolved
src/cheri-pte-ext.adoc Outdated Show resolved Hide resolved
src/cheri-pte-ext.adoc Outdated Show resolved Hide resolved
Co-authored-by: Alexander Richardson <[email protected]>
Signed-off-by: Tariq Kurd <[email protected]>
src/cheri-pte-ext.adoc Outdated Show resolved Hide resolved
src/cheri-pte-ext.adoc Outdated Show resolved Hide resolved
@tariqkurd-repo tariqkurd-repo merged commit c3781da into riscv:main Jan 15, 2025
3 checks passed
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.

there's no mechanism for separating kernel and usespace revocation sweeps
7 participants