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.

Sign up for GitHub

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

Added STIDC and UTIDC registers #247

Open
wants to merge 4 commits into
base: qemu-cheri
Choose a base branch
from
Open

Added STIDC and UTIDC registers #247

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
02032d2
Added STIDC and UTIDC registers
francislaus Apr 18, 2024
1fd9a10
Changed numbers for STIDC and UTIDC
francislaus Apr 19, 2024
986f0a1
Added MTIDC registers and enforced ASR only on writes for xTIDC regis…
francislaus May 8, 2024
d8b4544
Fixed error in is_write logic
francislaus May 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions target/riscv/cheri-archspecific-early.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,16 +80,19 @@ enum CheriSCR {
CheriSCR_PCC = 0,
CheriSCR_DDC = 1,

CheriSCR_UTIDC = 3,
CheriSCR_UTCC = 4,
CheriSCR_UTDC = 5,
CheriSCR_UScratchC = 6,
CheriSCR_UEPCC = 7,

CheriSCR_STIDC = 11,
CheriSCR_STCC = 12,
CheriSCR_STDC = 13,
CheriSCR_SScratchC = 14,
CheriSCR_SEPCC = 15,

CheriSCR_MTIDC = 27,
CheriSCR_MTCC = 28,
CheriSCR_MTDC = 29,
CheriSCR_MScratchC = 30,
Expand Down
3 changes: 3 additions & 0 deletions target/riscv/cpu.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -743,16 +743,19 @@ static void riscv_cpu_reset(DeviceState *dev)
null_capability(&env->UTDC);
null_capability(&env->UScratchC);
set_max_perms_capability(&env->UEPCC, 0);
null_capability(&env->UTIDC);
// Supervisor mode trap handling
set_max_perms_capability(&env->STCC, 0);
null_capability(&env->STDC);
null_capability(&env->SScratchC);
set_max_perms_capability(&env->SEPCC, 0);
null_capability(&env->STIDC);
// Machine mode trap handling
set_max_perms_capability(&env->MTCC, 0);
null_capability(&env->MTDC);
null_capability(&env->MScratchC);
set_max_perms_capability(&env->MEPCC, 0);
null_capability(&env->MTIDC);
#endif /* TARGET_CHERI */
#ifdef CONFIG_DEBUG_TCG
env->_pc_is_current = true;
Expand Down
3 changes: 3 additions & 0 deletions target/riscv/cpu.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -200,13 +200,15 @@ struct CPURISCVState {

#ifdef TARGET_CHERI
// XXX: not implemented properly
cap_register_t UTIDC; // SCR 3 User thread identifier cap. (UTIDC)
cap_register_t UTCC; // SCR 4 User trap code cap. (UTCC)
cap_register_t UTDC; // SCR 5 User trap data cap. (UTDC)
cap_register_t UScratchC; // SCR 6 User scratch cap. (UScratchC)
cap_register_t UEPCC; // SCR 7 User exception PC cap. (UEPCC)
#endif

#ifdef TARGET_CHERI
cap_register_t STIDC; // SCR 11 Supervisor thread identifier cap. (STIDC)
cap_register_t STCC; // SCR 12 Supervisor trap code cap. (STCC)
cap_register_t STDC; // SCR 13 Supervisor trap data cap. (STDC)
cap_register_t SScratchC; // SCR 14 Supervisor scratch cap. (SScratchC)
Expand All @@ -218,6 +220,7 @@ struct CPURISCVState {
target_ulong scause;

#ifdef TARGET_CHERI
cap_register_t MTIDC; // SCR 27 Machine thread identifier cap. (MTIDC)
cap_register_t MTCC; // SCR 28 Machine trap code cap. (MTCC)
cap_register_t MTDC; // SCR 29 Machine trap data cap. (MTDC)
cap_register_t MScratchC; // SCR 30 Machine scratch cap. (MScratchC)
Expand Down
38 changes: 29 additions & 9 deletions target/riscv/op_helper_cheri.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,25 +50,35 @@
enum SCRAccessMode {
SCR_Invalid = 0,
ASR_Flag = 1,
U_Always = (PRV_U + 1) << 1,
U_ASR = U_Always | ASR_Flag,
S_Always = (PRV_S + 1) << 1,
S_ASR = S_Always | ASR_Flag,
H_Always = (PRV_H + 1) << 1,
H_ASR = H_Always | ASR_Flag,
M_Always = (PRV_M + 1) << 1,
M_ASR = M_Always | ASR_Flag,
ASR_W_Flag = 2,
U_Always = (PRV_U + 1) << 2,
U_ASR_W = U_Always | ASR_W_Flag,
U_ASR = U_ASR_W | ASR_Flag,
Copy link
Member

Choose a reason for hiding this comment

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

U_ASR is ASR + ASR_W, which is a bit odd (i.e. you're inconsistent between the flag and the aliases what the unsuffixed version means)

Copy link
Member

Choose a reason for hiding this comment

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

Also, U_Always | ASR_Flag and U_Always | ASR_Flag | ASR_W_Flag behave the same, which is a bit confusing too IMO

Copy link
Author

Choose a reason for hiding this comment

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

@jrtc27 Would ASR_RW = ASR_R | ASR_W be something you are happy with? Then all SCRs other than xTIDC need to have ASR_RW.

S_Always = (PRV_S + 1) << 2,
S_ASR_W = S_Always | ASR_W_Flag,
S_ASR = S_ASR_W | ASR_Flag,
H_Always = (PRV_H + 1) << 2,
H_ASR_W = H_Always | ASR_W_Flag,
H_ASR = H_ASR_W | ASR_Flag,
M_Always = (PRV_M + 1) << 2,
M_ASR_W = M_Always | ASR_W_Flag,
M_ASR = M_ASR_W | ASR_Flag,
};

static inline int scr_min_priv(enum SCRAccessMode mode)
{
return ((int)mode >> 1) - 1;
return ((int)mode >> 2) - 1;
}
static inline int scr_needs_asr(enum SCRAccessMode mode)
{
return (mode & ASR_Flag) == ASR_Flag;
}

static inline int scr_needs_asr_w(enum SCRAccessMode mode)
{
return (mode & ASR_W_Flag) == ASR_W_Flag;
}

struct SCRInfo {
bool r;
bool w;
Expand All @@ -89,6 +99,7 @@ struct SCRInfo {
.access = U_ASR,
.name = "UScratchC"},
[CheriSCR_UEPCC] = {.r = true, .w = true, .access = U_ASR, .name = "UEPCC"},
[CheriSCR_UTIDC] = {.r = true, .w = true, .access = U_ASR_W, .name = "UTIDC"},

[CheriSCR_STCC] = {.r = true, .w = true, .access = S_ASR, .name = "STCC"},
[CheriSCR_STDC] = {.r = true, .w = true, .access = S_ASR, .name = "STDC"},
Expand All @@ -97,6 +108,7 @@ struct SCRInfo {
.access = S_ASR,
.name = "SScratchC"},
[CheriSCR_SEPCC] = {.r = true, .w = true, .access = S_ASR, .name = "SEPCC"},
[CheriSCR_STIDC] = {.r = true, .w = true, .access = S_ASR_W, .name = "STIDC"},

[CheriSCR_MTCC] = {.r = true, .w = true, .access = M_ASR, .name = "MTCC"},
[CheriSCR_MTDC] = {.r = true, .w = true, .access = M_ASR, .name = "MTDC"},
Expand All @@ -105,6 +117,7 @@ struct SCRInfo {
.access = M_ASR,
.name = "MScratchC"},
[CheriSCR_MEPCC] = {.r = true, .w = true, .access = M_ASR, .name = "MEPCC"},
[CheriSCR_MTIDC] = {.r = true, .w = true, .access = M_ASR_W, .name = "MTIDC"},

[CheriSCR_BSTCC] = {.r = true, .w = true, .access = H_ASR, .name= "BSTCC"},
[CheriSCR_BSTDC] = {.r = true, .w = true, .access = H_ASR, .name= "BSTCC"},
Expand All @@ -123,16 +136,19 @@ static inline cap_register_t *get_scr(CPUArchState *env, uint32_t index)
case CheriSCR_UTDC: return &env->UTDC;
case CheriSCR_UScratchC: return &env->UScratchC;
case CheriSCR_UEPCC: return &env->UEPCC;
case CheriSCR_UTIDC: return &env->UTIDC;

case CheriSCR_STCC: return &env->STCC;
case CheriSCR_STDC: return &env->STDC;
case CheriSCR_SScratchC: return &env->SScratchC;
case CheriSCR_SEPCC: return &env->SEPCC;
case CheriSCR_STIDC: return &env->STIDC;

case CheriSCR_MTCC: return &env->MTCC;
case CheriSCR_MTDC: return &env->MTDC;
case CheriSCR_MScratchC: return &env->MScratchC;
case CheriSCR_MEPCC: return &env->MEPCC;
case CheriSCR_MTIDC: return &env->MTIDC;

case CheriSCR_BSTCC: return &env->VSTCC;
case CheriSCR_BSTDC: return &env->VSTDC;
Expand Down Expand Up @@ -165,6 +181,10 @@ void HELPER(cspecialrw)(CPUArchState *env, uint32_t cd, uint32_t cs,
_host_return_address);
}
bool can_access_sysregs = cheri_have_access_sysregs(env);
bool is_write = (cs != 0);
Copy link
Member

Choose a reason for hiding this comment

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

Introducing a variable for this when there are multiple existing, untouched uses of cs != 0 isn't particularly clean

Copy link
Author

Choose a reason for hiding this comment

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

@jrtc27 The same would then apply to bool can_access_sysregs. I attempted to use the style of the code around, but I am more than happy to directly use cs != 0. I did not spot a variable that would give me the same semantics, but I might have missed it.

if (is_write && scr_needs_asr_w(mode) && !can_access_sysregs) {
raise_cheri_exception(env, CapEx_AccessSystemRegsViolation, 32 + index);
}
if (scr_needs_asr(mode) && !can_access_sysregs) {
raise_cheri_exception(env, CapEx_AccessSystemRegsViolation, 32 + index);
}
Expand Down
Loading