-
Notifications
You must be signed in to change notification settings - Fork 1k
Support Zicfiss (shadow stack access) with CFI extension v0.4.0 #1560
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
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 |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| #include "zicfiss.h" | ||
|
|
||
| if (xSSE()) { | ||
| POP_VALUE_FROM_SS_AND_CHECK(READ_REG(X_T0)); | ||
| } | ||
|
Contributor
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. could be: |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| #include "zicfiss.h" | ||
|
|
||
| if (xSSE()) { | ||
| PUSH_VALUE_TO_SS(RA); | ||
| } | ||
SuHo-llrr marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| require_extension(EXT_ZICFISS); | ||
| require_extension('A'); | ||
| require_rv64; | ||
|
|
||
| DECLARE_XENVCFG_VARS(SSE); | ||
| require_envcfg(SSE); | ||
| WRITE_RD(MMU.ssamoswap<uint64_t>(RS1, RS2)); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| require_extension(EXT_ZICFISS); | ||
| require_extension('A'); | ||
|
|
||
| DECLARE_XENVCFG_VARS(SSE); | ||
| require_envcfg(SSE); | ||
| WRITE_RD(sext32(MMU.ssamoswap<uint32_t>(RS1, RS2))); | ||
|
|
||
SuHo-llrr marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| #include "zicfiss.h" | ||
|
|
||
| if (xSSE()) { | ||
| POP_VALUE_FROM_SS_AND_CHECK(RS1); | ||
| } else { | ||
| #include "mop_r_N.h" | ||
| } | ||
SuHo-llrr marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| #include "sspopchk_x1.h" |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| #include "zicfiss.h" | ||
|
|
||
|
Contributor
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. Is the require_extension(EXT_ZICFISS) required? If extension is not present the instructions turn into mops and that is already handled by xSSE? The require_extension will cause an illegal-instruction to be thrown...
Contributor
Author
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 you are right ! xSSE() has already implied whether B is enabled. |
||
| if (xSSE()) { | ||
| PUSH_VALUE_TO_SS(RS2); | ||
| } else { | ||
| #include "mop_rr_N.h" | ||
| } | ||
SuHo-llrr marked this conversation as resolved.
Show resolved
Hide resolved
Contributor
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 small duplication of code between compressed and uncompressed sspush and sspopchk seems unfortunate. Since we now have a zicfiss.h is it possible to have a sspush and a sspopchk macro that can be invoked by the 3 sspush and 3 sspopchk respectively?
Contributor
Author
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. After refactoring the code, I think I can organize it into Thanks for the suggestion |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| #include "sspush_x1.h" |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| #include "zicfiss.h" | ||
|
|
||
| if (xSSE()) { | ||
| WRITE_RD(STATE.ssp->read()); | ||
| } else { | ||
| #include "mop_r_N.h" | ||
| } | ||
SuHo-llrr marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -42,9 +42,10 @@ struct xlate_flags_t { | |
| const bool forced_virt : 1 {false}; | ||
| const bool hlvx : 1 {false}; | ||
| const bool lr : 1 {false}; | ||
| const bool ss_access : 1 {false}; | ||
|
|
||
| bool is_special_access() const { | ||
| return forced_virt || hlvx || lr; | ||
| return forced_virt || hlvx || lr || ss_access; | ||
| } | ||
| }; | ||
|
|
||
|
|
@@ -127,6 +128,14 @@ class mmu_t | |
| return load<T>(addr, {.forced_virt=true, .hlvx=true}); | ||
| } | ||
|
|
||
| // shadow stack load | ||
| template<typename T> | ||
| T ss_load(reg_t addr) { | ||
| if ((addr & (sizeof(T) - 1)) != 0) | ||
| throw trap_store_access_fault((proc) ? proc->state.v : false, addr, 0, 0); | ||
| return load<T>(addr, {.forced_virt=false, .hlvx=false, .lr=false, .ss_access=true}); | ||
| } | ||
|
|
||
| template<typename T> | ||
| void ALWAYS_INLINE store(reg_t addr, T val, xlate_flags_t xlate_flags = {}) { | ||
| reg_t vpn = addr >> PGSHIFT; | ||
|
|
@@ -149,6 +158,14 @@ class mmu_t | |
| store(addr, val, {.forced_virt=true}); | ||
| } | ||
|
|
||
| // shadow stack store | ||
| template<typename T> | ||
| void ss_store(reg_t addr, T val) { | ||
| if ((addr & (sizeof(T) - 1)) != 0) | ||
| throw trap_store_access_fault((proc) ? proc->state.v : false, addr, 0, 0); | ||
| store<T>(addr, val, {.forced_virt=false, .hlvx=false, .lr=false, .ss_access=true}); | ||
| } | ||
|
|
||
| // AMO/Zicbom faults should be reported as store faults | ||
| #define convert_load_traps_to_store_traps(BODY) \ | ||
| try { \ | ||
|
|
@@ -175,6 +192,19 @@ class mmu_t | |
| }) | ||
| } | ||
|
|
||
| // for shadow stack amoswap | ||
| template<typename T> | ||
| T ssamoswap(reg_t addr, reg_t value) { | ||
| bool forced_virt = false; | ||
| bool hlvx = false; | ||
| bool lr = false; | ||
| bool ss_access = true; | ||
| store_slow_path(addr, sizeof(T), nullptr, {forced_virt, hlvx, lr, ss_access}, false, true); | ||
|
Contributor
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. Do we need to do alignment check here? ssamoswap requires address to be naturally aligned to size.
Contributor
Author
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. store_slow_path already check required_aligned parameter with true
Contributor
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. Thanks. If |
||
| auto data = load<T>(addr, {forced_virt, hlvx, lr, ss_access}); | ||
| store<T>(addr, value, {forced_virt, hlvx, lr, ss_access}); | ||
| return data; | ||
| } | ||
|
|
||
SuHo-llrr marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| template<typename T> | ||
| T amo_compare_and_swap(reg_t addr, T comp, T swap) { | ||
| convert_load_traps_to_store_traps({ | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -22,3 +22,10 @@ DECLARE_OVERLAP_INSN(rstsa32, EXT_ZPN) | |
| DECLARE_OVERLAP_INSN(srli32_u, EXT_ZPN) | ||
| DECLARE_OVERLAP_INSN(umax32, EXT_ZPN) | ||
| DECLARE_OVERLAP_INSN(lpad, EXT_ZICFILP) | ||
| DECLARE_OVERLAP_INSN(mop_r_28, EXT_ZIMOP) | ||
SuHo-llrr marked this conversation as resolved.
Show resolved
Hide resolved
Contributor
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 realized we should also declare overlap for mop_rr_7 which is also used by Zicfiss. |
||
| DECLARE_OVERLAP_INSN(mop_r_N, EXT_ZIMOP) | ||
| DECLARE_OVERLAP_INSN(mop_rr_7, EXT_ZIMOP) | ||
| DECLARE_OVERLAP_INSN(mop_rr_N, EXT_ZIMOP) | ||
| DECLARE_OVERLAP_INSN(c_sspush_x1, EXT_ZICFISS) | ||
| DECLARE_OVERLAP_INSN(c_sspopchk_x5, EXT_ZICFISS) | ||
| DECLARE_OVERLAP_INSN(c_mop_N, EXT_ZCMOP) | ||
Uh oh!
There was an error while loading. Please reload this page.