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

Indirect Sentries #51

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

Indirect Sentries #51

wants to merge 10 commits into from

Conversation

nwf
Copy link
Member

@nwf nwf commented Sep 29, 2021

This sits on top of #18 at the moment, just because it needs to read capabilities from memory.

src/cheri_insts.sail Outdated Show resolved Hide resolved
src/cheri_types.sail Outdated Show resolved Hide resolved
src/cheri_insts.sail Outdated Show resolved Hide resolved
src/cheri_insts.sail Outdated Show resolved Hide resolved
@nwf nwf requested a review from jrtc27 September 30, 2021 02:33
@nwf nwf changed the title WIP: Indirect sentries Points-to-PCC (non-pair) Indirect Sentries Oct 27, 2021
@nwf nwf force-pushed the 202109-isentry branch 4 times, most recently from 69acf10 to 0a2c8a7 Compare November 1, 2021 13:59
@nwf nwf changed the title Points-to-PCC (non-pair) Indirect Sentries Indirect Sentries Nov 1, 2021
nwf-msr and others added 9 commits November 2, 2021 16:20
The cap-store instructions now look at the data being stored to decide
whether they issue Write(Cap) or Write(Data) requests on "the bus".
This allows the PTW logic (and update_PTE_Bits) in particular to not
fast-cap-dirty a page that's being targeted by the store of an untagged
capability.  This is not, however, viable for AMOCAS and so additional
changes may be required if we are to avoid considering AMOCAS as always
capability-dirtying.

Co-authored-by: Jessica Clarke <[email protected]>
Use Either-monadic style

Eliminate ext_ptw_sc / PTW_SC_* as we now simply test the error cases in
priority order, so there's no need to explicitly pass this information forward
in checkPTEPermission.  ext_ptw_lc / PTW_LC_* persist as the extended PTW
information is analysed by instructions.

Co-authored-by: Jessica Clarke <[email protected]>
Refactor the test for ambient sealing out to its own function, as we'll
want to extend it more generally with indirect sentries.  Allow this
function to (request to) raise CapEx exceptions as well and have it flag
a PermitExecuteViolation for sentries without permit_execute.

Co-Authored-By: Alexander Richardson <[email protected]>
Common up with handle_loadres_cap_via_cap.
* ## Exceptions
* Takes the link target register index, cd; the source register index, cs (used
* only when signaling an exception); and the Capability to be installed as
* PCC. Note that C(cs)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Incomplete sentence?

* - *cs1*.**address** $+$ *imm* $+$ min_instruction_bytes $\gt$ *cs1*.**top**.
* - *cs1*.**base** is unaligned.
* - *cs1*.**address** $+$ *imm* is unaligned, ignoring bit 0.
* Either sets the CPU into an exception state and returns fail or sets the CPU
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* Either sets the CPU into an exception state and returns fail or sets the CPU
* Either sets the CPU into an exception state and returns failure or sets the CPU

returns fail sounds a bit odd to me.

* - *cs1*.**address** $+$ *imm* is unaligned, ignoring bit 0.
*/
function clause execute(CJALR(imm, cs1, cd)) = {
cjalr_worker(cd, cs1, C(cs1), EXTS(imm))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We will probably have to update the spec to include a section with helper functions?

/*
* Indirect sentries.
*
* At the risk of becoming CISC, the invocation instructions load, unseal, and
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* At the risk of becoming CISC, the invocation instructions load, unseal, and
* The invocation instructions load, unseal, and

* ## Exceptions
*
* An exception is raised if:
* - *cs1*.**tag** is not set.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We probably want to update this behaviour to match #56. Although for this instruction I'm less worried about the trapping behaviour

* - *cs1*.**perms** does not grant both **Permit_Load** and
* **Permit_Load_Capability**.
*/
function clause execute (CSealL(cd, cs1)) = {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure about calling it CSealL, maybe CSealIL/CSealInd/CSealI to emphasize that these sealed capabilities are indirect?

}
}

union clause ast = CInvokeLAL : (regidx, regidx)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what does LAL stand for?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the intention was "Load And Link", though I no longer remember. It's a bad name.

@@ -63,6 +63,8 @@
let reserved_otypes = 4
let otype_unsealed = -1
let otype_sentry = -2 /* Sealed erstwhile or prospective PCC */
let otype_isentry_load = -3 /* Sealed erstwhile IDC pointing at PCC */
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is erstwhile correct? Isn't this what is going to be idc when invoked rather than what was idc when sealed?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How embarrassing.

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.

4 participants