-
Notifications
You must be signed in to change notification settings - Fork 50
Support for Smepmp #652
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
base: main
Are you sure you want to change the base?
Support for Smepmp #652
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,9 +6,110 @@ name: mseccfg | |
long_name: Machine Security Configuration | ||
address: 0x747 | ||
priv_mode: M | ||
length: 64 | ||
description: Machine Security Configuration | ||
length: MXLEN | ||
description: Machine Security Configuration register is used for configuring various security mechanisms present on the hart and only accessible in Machine mode. | ||
definedBy: | ||
name: Sm | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why are we removing Sm here? There is no way we can implement this without M-Mode There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I actually added defined by both the Sm and Smepmp. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My bad, you are right, you did. The same comment then applies to both definedBy, but let's have Paul take a look at it There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The spec looks like the just dumped the "Smepmp" extension proposal into the doc, as 6.2 is literally titled "Proposal". :-/ Anyway, that section says:
Based on that, Smepmp defined the CSR, and should be the only one listed here. "definedBy" is serving a different purpose than "requires"... just the extension that literally defines the CSR. |
||
version: ">= 1.12" | ||
fields: {} | ||
allOf: | ||
- name: Sm | ||
version: ">=1.12" | ||
- name: Smepmp | ||
version: ">= 1.0.0" | ||
fields: | ||
MML: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See comments for MMWP; similar situation for this bit. |
||
location: 0 | ||
description: | | ||
Machine Mode Lockdown (mseccfg.MML) enforces strong isolation between Machine Mode and lower-privilege modes. This is a _sticky bit_ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Where did this sentence come from? A general guideline is to use text from the spec and only text from the spec, preferably unmodified, if possible, to provide content that is accurate and complete. Reading ahead, the content differs from the spec in some minor areas, or we're looking at different specs. I'm looking at _The RISC-V Instruction Set Manual: Volume II: Privileged Architecture" dated 20240411. I think this is the lastest version... please use the text from the latest version. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I actually was confused whether to add a summary of the orignal spec or completely copy/paste everything. But, now onward, in the next commit, I will replace with the original spec content.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. My preference is text directly from the spec that fully describes the extension and does not belong elsewhere. So, descriptions of instructions, CSRs/fields is not needed in the extension, because that content goes in other YAML files. |
||
meaning that once set, it can only be reset on PMP Reset. | ||
When `mseccfg.MML` is 1 (set), it redefines: | ||
a). `pmpcfg.L` bit as: | ||
- When `pmpcfg.L` is set, PMP rules are *enforced* on M-Mode only and *denied* on S/U-modes. | ||
- When `pmpcfg.L` is unset, PMP rules are *enforced* on S/U-modes-only and *denied* on M-mode. | ||
Formerly Reserved Encodings `pmpcfg.RW=01` and the encoding `pmpcfg.LRWX=1111` as *Shared-Region*. | ||
A _Shared region Rule_ is *enforced* on all modes, with restrictions depending on the `pmpcfg.L` and `pmpcfg.X` bits as: | ||
- A _Shared-Region rule_ where `pmpcfg.L` is not set can be used for sharing data between M-mode and | ||
S/U-mode, so is not executable. M-mode has read/write access to that region, and S/U-mode has | ||
read access if `pmpcfg.X` is not set, or read/write access if `pmpcfg.X` is set. | ||
- A Shared-Region rule where `pmpcfg.L` is set can be used for sharing code between M-mode and | ||
S/U-mode, so is not writeable. Both M-mode and S/U-mode have execute access on the region, and | ||
M-mode also has read access if `pmpcfg.X` is set. The rule remains locked so that any further | ||
modifications to its associated configuration or address registers are ignored until a PMP reset, | ||
unless `mseccfg.RLB` is set. | ||
- The encoding `pmpcfg.LRWX=1111` can be used for sharing data between M-mode and S/U mode, | ||
where both modes only have read-only access to the region. The rule remains locked so that any | ||
further modifications to its associated configuration or address registers are ignored until a PMP | ||
reset, unless `mseccfg.RLB` is set. | ||
b). Adding a rule with executable privileges that either is M-mode-only or a locked Shared-Region is not | ||
possible and such `pmpcfg` writes are ignored, leaving `pmpcfg` unchanged. This restriction can be | ||
temporarily lifted e.g. during the boot process, by setting mseccfg.RLB. | ||
c). Executing code with Machine mode privileges is only possible from memory regions with a matching M- | ||
mode-only rule or a locked Shared-Region rule with executable privileges. Executing code from a | ||
region without a matching rule or with a matching S/U-mode-only rule is *denied*. | ||
d). If `mseccfg.MML` is not set, the combination of `pmpcfg.RW=01` remains reserved for future standard use. | ||
The truth table when the `mseccfg.MML` is set: | ||
[cols="4*^.^1,2*^.^3", separator="!", %autowidth, options="header"] | ||
!==== | ||
4+^! Bits on _pmpcfg_ register 2+^! Result | ||
! L ! R ! W ! X ! M Mode ! S/U Mode | ||
! 0 ! 0 ! 0 ! 0 2+^! Inaccessible region (Access Exception) | ||
! 0 ! 0 ! 0 ! 1 ! Access Exception ! Execute-only region | ||
! 0 ! 0 ! 1 ! 0 2+^! Shared data region: Read/write on M mode, Read-only on S/U mode | ||
! 0 ! 0 ! 1 ! 1 2+^! Shared data region: Read/write for both M and S/U mode | ||
! 0 ! 1 ! 0 ! 0 ! Access Exception ! Read-only region | ||
! 0 ! 1 ! 0 ! 1 ! Access Exception ! Read/Execute region | ||
! 0 ! 1 ! 1 ! 0 ! Access Exception ! Read/Write region | ||
! 0 ! 1 ! 1 ! 1 ! Access Exception ! Read/Write/Execute region | ||
! 1 ! 0 ! 0 ! 0 2+^! Locked inaccessible region* (Access Exception) | ||
! 1 ! 0 ! 0 ! 1 ! Locked Execute-only region* ! Access Exception | ||
! 1 ! 0 ! 1 ! 0 2+^! Locked Shared code region: Execute only on both M and S/U mode.* | ||
! 1 ! 0 ! 1 ! 1 2+^! Locked Shared code region: Execute only on S/U mode, read/execute on M mode.* | ||
! 1 ! 1 ! 0 ! 0 ! Locked Read-only region* ! Access Exception | ||
! 1 ! 1 ! 0 ! 1 ! Locked Read/Execute region* ! Access Exception | ||
! 1 ! 1 ! 1 ! 0 ! Locked Read/Write region* ! Access Exception | ||
! 1 ! 1 ! 1 ! 1 2+^! Locked Shared data region: Read only on both M and S/U mode.* | ||
!==== | ||
*Locked rules cannot be removed or modified until a PMP reset, unless mseccfg.RLB is set. | ||
type: RW | ||
definedBy: Smepmp | ||
reset_value: UNDEFINED_LEGAL | ||
MMWP: | ||
location: 1 | ||
description: | | ||
Machine Mode Whitelist Policy (mseccfg.MMWP). This is a _sticky bit_ meaning that once set, it can only be reset on PMP Reset. | ||
When 1 (set), it changes the default PMP policy for M-mode when accessing memory regions that do not have a matching PMP rule, to | ||
*denied* instead of *ignored*. | ||
When set to 0, `mseccfg.MMWP` enables the default PMP behavior in Machine mode, meaning that M-mode can access any memory region | ||
even if it is not explicitly covered by a PMP rule. | ||
type: RW | ||
definedBy: Smepmp | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
parameters:
MSECCFG_MMWP_TYPE:
schema:
enum: [read-only-0, read-only-1, reset-to-0, reset-to-1]
description: |
Determines the behavior of the mseccfg.MMWP bit:
* "read-only-0": Bit is hardwired to 0
* "read-only-1": Bit is hardwired to 1
* "reset-to-0": Bit comes out of reset cleared, and can be set once before becoming sticky The type of the field is parameter dependent, e.g.,: type(): |
if (MSECCFG_MMWP_TYPE == "read-only-0") {
return CsrFieldType::RO;
} else if (...) And there needs to a sw_write function since the bit is sticky: sw_write(csr_value): return csr_value.MMWP | CSR[mseccfg].MMWP; |
||
reset_value: UNDEFINED_LEGAL | ||
RLB: | ||
location: 2 | ||
description: | | ||
Rule Locking Bypass (mseccfg.RLB). This field can be set to 1 and once it is set back to 0, then it cannot be changed until the PMP Reset. | ||
When 1, locked PMP rules may be removed/modified and locked PMP enteries may be edited. | ||
When 0, with `pmpcfg.L=1` in any rule or entry (including disabled enteries), then mseccfg.RLB | ||
remains 0 and any further modifications to mseccfg.RLB are ignored until a PMP reset. | ||
type: RW | ||
definedBy: Smepmp | ||
reset_value: UNDEFINED_LEGAL | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Comment on lines
+105
to
+115
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
For example Smepmp.yaml parameters:
MUTABLE_MSECCFG_RLB:
schema:
type: boolean
description: |
When set, mseccfg.RLB is writable.
When clear, mseccfg.RLB is read-only-0. mseccfg.yaml # instead of `type: RW`
type(): return MUTABLE_MSECCFG_RLB ? CsrFieldType::RW : CsrFieldType:RO; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,6 +10,9 @@ priv_mode: M | |
length: 32 | ||
description: Machine Security Configuration | ||
definedBy: | ||
name: Sm | ||
version: ">= 1.12" | ||
allOf: | ||
- name: Sm | ||
version: ">=1.12" | ||
- name: Smepmp | ||
version: ">= 1.0.0" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am not super sure about this. Obviously, we need machine mode to have Smepmp, but isn't that a relationship in-between the extensions rather than something this CSR should define? Likely @ThinkOpenly has a better view of this than me There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, "Smepmp" only, please. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also, it's better to use the "compatible" operator for the version requirement here: - name: Smepmp
version: ~> 1.0.0 That's equivalent to |
||
fields: {} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,57 @@ | ||
# yaml-language-server: $schema=../../schemas/ext_schema.json | ||
|
||
$schema: "ext_schema.json#" | ||
kind: extension | ||
name: Smepmp | ||
type: privileged | ||
long_name: Personal Physical Memory Protection (PMP) Enhancements for memory access and execution prevention on Machine mode | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Personal should be removed and I don't think there is a need to redefine There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry, personal was added by mistake. Sure, I will update this. |
||
versions: | ||
- version: "1.0.0" | ||
state: ratified | ||
ratification_date: 2021-12 | ||
contributors: | ||
- name: Nick Kossifidis | ||
- name: Joe Xie | ||
- name: Bill Huffman | ||
- name: Allen Baum | ||
- name: Greg Favor | ||
- name: Tariq Kurd | ||
- name: Fumio Arakawa | ||
- name: RISC-V TEE Task Group | ||
|
||
description: | | ||
Being able to access the memory of a process running at a high privileged execution mode, such as the | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here, we should take much more of the spec rather just the introductory paragraph, right? If things like the threat model are not described here, they won't be anywhere else There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure, I will add everything from the spec. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There are somethings which may be repetitions of the CSRs, but we definitely need more than this. Also, there is an image we should also add, can you add it? I am not sure what the process for that is There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, I was also thinking about the image and was not sure, whether we should add this or not. But, I will add that in the next commit and move/remove that after the review. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure if we already did add some image, it may be something not possible to do yet, but give it a try and let me know. If it needs some extra ruby code, I can help you. I don't think it is fully required to merge this PR since it is a bit out of your scope There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This gets in to the topic of what we are trying to describe in the extension YAML files. Two directions:
We have both intermixed right now in the data. This is a UDB SIG topic |
||
Supervisor or Machine mode, from a lower privileged mode such as the User mode, introduces an obvious attack | ||
vector since it allows for an attacker to perform privilege escalation, and tamper with the code and/or data of | ||
that process. A less obvious attack vector exists when the reverse happens, in which case an attacker instead of | ||
tampering with code and/or data that belong to a high-privileged process, can tamper with the memory of an | ||
unprivileged / less-privileged process and trick the high-privileged process to use or execute it. | ||
To prevent this attack vector, two mechanisms known as Supervisor Memory Access Prevention (SMAP) and | ||
Supervisor Memory Execution Prevention (SMEP) were introduced in recent systems. The first one prevents the | ||
OS from accessing the memory of an unprivileged process unless a specific code path is followed, and the second | ||
one prevents the OS from executing the memory of an unprivileged process at all times. RISC-V already includes | ||
support for SMAP, through the sstatus.SUM bit, and for SMEP by always denying execution of virtual memory | ||
pages marked with the U bit, with Supervisor mode (OS) privileges, as mandated on the Privilege Spec. | ||
[NOTE] | ||
-- | ||
Terms: | ||
- *PMP Entry*: A pair of `pmpcfg[i]` / `pmpaddr[i]` registers. | ||
- *PMP Rule*: The contents of a pmpcfg register and its associated pmpaddr register(s), | ||
that encode a valid protected physical memory region, where `pmpcfg[i].A != OFF`, and | ||
if `pmpcfg[i].A == TOR`, `pmpaddr[i-1] < pmpaddr[i]`. | ||
- *Ignored*: Any permissions set by a matching PMP rule are ignored, and all accesses to | ||
the requested address range are allowed. | ||
- *Enforced*: Only access types configured in the PMP rule matching the requested address | ||
range are allowed; failures will cause an access-fault exception. | ||
- *Denied*: Any permissions set by a matching PMP rule are ignored, and no accesses to the | ||
requested address range are allowed.; failures will cause an access-fault exception. | ||
- *Locked*: A PMP rule/entry where the `pmpcfg.L` bit is set. | ||
- *PMP reset*: A reset process where all PMP settings of the hart, including locked | ||
rules/settings, are re-initialized to a set of safe defaults, before releasing the hart (back) | ||
to the firmware / OS / application. | ||
Comment on lines
+40
to
+56
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this is for the glossary, which @kersten1 is working on separately. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. okay, I will remove it. |
||
-- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove "and only accessible in Machine Mode", since it's redundant with the "priv_mode", above.