Skip to content

Commit 0fc627f

Browse files
nemecadnemecad
committed
Machine: propagate xRET type and add privilege flags illegal instruction checks
Decode MRET/SRET/URET in the decode stage, carry the return type through the interstage registers, and pass it to ControlState::exception_return in the memory stage. Extend instruction metadata with privilege flags (IMF_PRIV_M/H/S) for privileged operations and use them for masking.
1 parent ee811fa commit 0fc627f

File tree

7 files changed

+105
-18
lines changed

7 files changed

+105
-18
lines changed

‎src/machine/core.cpp‎

Lines changed: 40 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,27 @@ Xlen Core::get_xlen() const {
127127

128128
void Core::set_current_privilege(CSR::PrivilegeLevel privilege) {
129129
state.set_current_privilege(privilege);
130+
const unsigned PRIV_BITS = unsigned(IMF_PRIV_M) | unsigned(IMF_PRIV_H) | unsigned(IMF_PRIV_S);
131+
132+
InstructionFlags base_mask = InstructionFlags(unsigned(check_inst_flags_mask) & ~PRIV_BITS);
133+
InstructionFlags base_val = InstructionFlags(unsigned(check_inst_flags_val) & ~PRIV_BITS);
134+
135+
unsigned allowed_priv = 0;
136+
if (privilege >= CSR::PrivilegeLevel::SUPERVISOR) {
137+
allowed_priv |= unsigned(IMF_PRIV_S);
138+
}
139+
if (privilege >= CSR::PrivilegeLevel::HYPERVISOR) {
140+
allowed_priv |= unsigned(IMF_PRIV_H);
141+
}
142+
if (privilege >= CSR::PrivilegeLevel::MACHINE) {
143+
allowed_priv |= unsigned(IMF_PRIV_M);
144+
}
145+
unsigned disallowed_priv = (PRIV_BITS & ~allowed_priv);
146+
InstructionFlags new_mask = InstructionFlags(unsigned(base_mask) | disallowed_priv);
147+
InstructionFlags new_val = base_val;
148+
149+
check_inst_flags_mask = new_mask;
150+
check_inst_flags_val = new_val;
130151
}
131152

132153
CSR::PrivilegeLevel Core::get_current_privilege() const {
@@ -317,7 +338,18 @@ DecodeState Core::decode(const FetchInterstage &dt) {
317338
ExceptionCause excause = dt.excause;
318339

319340
dt.inst.flags_alu_op_mem_ctl(flags, alu_op, mem_ctl);
320-
341+
CSR::PrivilegeLevel inst_xret_priv = CSR::PrivilegeLevel::UNPRIVILEGED;
342+
if (flags & IMF_XRET) {
343+
if (flags & IMF_PRIV_M) {
344+
inst_xret_priv = CSR::PrivilegeLevel::MACHINE;
345+
} else if (flags & IMF_PRIV_H) {
346+
inst_xret_priv = CSR::PrivilegeLevel::HYPERVISOR;
347+
} else if (flags & IMF_PRIV_S) {
348+
inst_xret_priv = CSR::PrivilegeLevel::SUPERVISOR;
349+
} else {
350+
inst_xret_priv = CSR::PrivilegeLevel::UNPRIVILEGED;
351+
}
352+
}
321353
if ((flags ^ check_inst_flags_val) & check_inst_flags_mask) { excause = EXCAUSE_INSN_ILLEGAL; }
322354

323355
RegisterId num_rs = (flags & (IMF_ALU_REQ_RS | IMF_ALU_RS_ID)) ? dt.inst.rs() : 0;
@@ -333,7 +365,7 @@ DecodeState Core::decode(const FetchInterstage &dt) {
333365

334366
CSR::Address csr_address = (flags & IMF_CSR) ? dt.inst.csr_address() : CSR::Address(0);
335367
RegisterValue csr_read_val
336-
= ((control_state != nullptr && (flags & IMF_CSR))) ? control_state->read(csr_address) : 0;
368+
= ((control_state != nullptr && (flags & IMF_CSR))) ? control_state->read(csr_address, get_current_privilege()) : 0;
337369
bool csr_write = (flags & IMF_CSR) && (!(flags & IMF_CSR_TO_ALU) || (num_rs != 0));
338370

339371
if ((flags & IMF_EXCEPTION) && (excause == EXCAUSE_NONE)) {
@@ -393,6 +425,7 @@ DecodeState Core::decode(const FetchInterstage &dt) {
393425
.csr_to_alu = bool(flags & IMF_CSR_TO_ALU),
394426
.csr_write = csr_write,
395427
.xret = bool(flags & IMF_XRET),
428+
.xret_privlev = inst_xret_priv,
396429
.insert_stall_before = bool(flags & IMF_CSR) } };
397430
}
398431

@@ -463,6 +496,7 @@ ExecuteState Core::execute(const DecodeInterstage &dt) {
463496
.csr = dt.csr,
464497
.csr_write = dt.csr_write,
465498
.xret = dt.xret,
499+
.xret_privlev = dt.xret_privlev,
466500
} };
467501
}
468502

@@ -523,11 +557,12 @@ MemoryState Core::memory(const ExecuteInterstage &dt) {
523557
if (control_state != nullptr && dt.is_valid && dt.excause == EXCAUSE_NONE) {
524558
control_state->increment_internal(CSR::Id::MINSTRET, 1);
525559
if (dt.csr_write) {
526-
control_state->write(dt.csr_address, dt.alu_val);
560+
control_state->write(dt.csr_address, dt.alu_val, get_current_privilege());
527561
csr_written = true;
528562
}
529563
if (dt.xret) {
530-
CSR::PrivilegeLevel restored = control_state->exception_return(get_current_privilege());
564+
CSR::PrivilegeLevel restored =
565+
control_state->exception_return(get_current_privilege(), dt.xret_privlev);
531566
set_current_privilege(restored);
532567
if (this->xlen == Xlen::_32)
533568
computed_next_inst_addr
@@ -577,6 +612,7 @@ MemoryState Core::memory(const ExecuteInterstage &dt) {
577612
.regwrite = regwrite,
578613
.is_valid = dt.is_valid,
579614
.csr_written = csr_written,
615+
.xret_privlev = dt.xret_privlev,
580616
} };
581617
}
582618

‎src/machine/core.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -115,8 +115,8 @@ class Core : public QObject {
115115
Address mem_ref_addr);
116116

117117
const Xlen xlen;
118-
const InstructionFlags check_inst_flags_val;
119-
const InstructionFlags check_inst_flags_mask;
118+
InstructionFlags check_inst_flags_val;
119+
InstructionFlags check_inst_flags_mask;
120120
BORROWED Registers *const regs;
121121
BORROWED CSR::ControlState *const control_state;
122122
BORROWED BranchPredictor *const predictor;

‎src/machine/csr/controlstate.cpp‎

Lines changed: 27 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -55,16 +55,22 @@ namespace machine { namespace CSR {
5555
}
5656
}
5757

58-
RegisterValue ControlState::read(Address address) const {
59-
// Only machine level privilege is supported so no checking is needed.
58+
RegisterValue ControlState::read(Address address, PrivilegeLevel current_priv) const {
6059
size_t reg_id = get_register_internal_id(address);
60+
PrivilegeLevel required = address.get_privilege_level();
61+
if (current_priv < required) {
62+
throw SIMULATOR_EXCEPTION(
63+
UnsupportedInstruction,
64+
QString("CSR address %1 not accessible at current privilege level.").arg(address.data),
65+
"");
66+
}
6167
RegisterValue value = register_data[reg_id];
6268
DEBUG("Read CSR[%u] == 0x%" PRIx64, address.data, value.as_u64());
6369
emit read_signal(reg_id, value);
6470
return value;
6571
}
6672

67-
void ControlState::write(Address address, RegisterValue value) {
73+
void ControlState::write(Address address, RegisterValue value, PrivilegeLevel current_priv) {
6874
DEBUG(
6975
"Write CSR[%u/%zu] <== 0x%zu", address.data, get_register_internal_id(address),
7076
value.as_u64());
@@ -74,6 +80,15 @@ namespace machine { namespace CSR {
7480
UnsupportedInstruction,
7581
QString("CSR address %1 is not writable.").arg(address.data), "");
7682
}
83+
84+
PrivilegeLevel required = address.get_privilege_level();
85+
if (current_priv < required) {
86+
throw SIMULATOR_EXCEPTION(
87+
UnsupportedInstruction,
88+
QString("CSR address %1 not writable at current privilege level.").arg(address.data),
89+
"");
90+
}
91+
7792
write_internal(get_register_internal_id(address), value);
7893
}
7994

@@ -190,11 +205,11 @@ namespace machine { namespace CSR {
190205
emit write_signal(reg_id, reg);
191206
}
192207

193-
PrivilegeLevel ControlState::exception_return(enum PrivilegeLevel act_privlev) {
208+
PrivilegeLevel ControlState::exception_return(enum PrivilegeLevel act_privlev, enum PrivilegeLevel xret_privlev) {
194209
size_t reg_id = Id::MSTATUS;
195210
RegisterValue &reg = register_data[reg_id];
196211
PrivilegeLevel restored_privlev = PrivilegeLevel::MACHINE;
197-
if (act_privlev == PrivilegeLevel::MACHINE) {
212+
if (xret_privlev == PrivilegeLevel::MACHINE) {
198213
// MRET semantics:
199214
// MIE <- MPIE
200215
// MPIE <- 1
@@ -212,7 +227,7 @@ namespace machine { namespace CSR {
212227
default: restored_privlev = PrivilegeLevel::UNPRIVILEGED; break;
213228
}
214229
write_field(Field::mstatus::MPP, (uint64_t)0); // clear MPP per spec
215-
} else if (act_privlev == PrivilegeLevel::SUPERVISOR) {
230+
} else if (xret_privlev == PrivilegeLevel::SUPERVISOR) {
216231
// SRET semantics:
217232
// SIE <- SPIE
218233
// SPIE <- 1
@@ -229,6 +244,12 @@ namespace machine { namespace CSR {
229244
restored_privlev = PrivilegeLevel::UNPRIVILEGED;
230245
}
231246

247+
// If the instruction was executed in M-mode and the restored privilege is less-privileged
248+
// than M, clear MPRV per the privileged spec.
249+
if (act_privlev == PrivilegeLevel::MACHINE && restored_privlev != PrivilegeLevel::MACHINE) {
250+
write_field(Field::mstatus::MPRV, (uint64_t)0);
251+
}
252+
232253
emit write_signal(reg_id, reg);
233254

234255
return restored_privlev;

‎src/machine/csr/controlstate.h‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ namespace machine { namespace CSR {
9999
ControlState(const ControlState &);
100100

101101
/** Read CSR register with ISA specified address. */
102-
[[nodiscard]] RegisterValue read(Address address) const;
102+
[[nodiscard]] RegisterValue read(Address address, PrivilegeLevel current_priv) const;
103103

104104
/**
105105
* Read CSR register with an internal id.
@@ -110,7 +110,7 @@ namespace machine { namespace CSR {
110110
[[nodiscard]] RegisterValue read_internal(size_t internal_id) const;
111111

112112
/** Write value to CSR register by ISA specified address and receive the previous value. */
113-
void write(Address address, RegisterValue value);
113+
void write(Address address, RegisterValue value, PrivilegeLevel current_priv);
114114

115115
/** Used for writes occurring as a side-effect (instruction count update...) and
116116
* internally by the write method. */
@@ -150,7 +150,7 @@ namespace machine { namespace CSR {
150150
public slots:
151151
void set_interrupt_signal(uint irq_num, bool active);
152152
void exception_initiate(PrivilegeLevel act_privlev, PrivilegeLevel to_privlev);
153-
PrivilegeLevel exception_return(enum PrivilegeLevel act_privlev);
153+
PrivilegeLevel exception_return(enum PrivilegeLevel act_privlev, enum PrivilegeLevel xret_privlev);
154154

155155
private:
156156
static size_t get_register_internal_id(Address address);
@@ -211,6 +211,7 @@ namespace machine { namespace CSR {
211211
static constexpr RegisterFieldDesc MIE = { "MIE", Id::MSTATUS, {1, 3}, "Machine global interrupt-enable"};
212212
static constexpr RegisterFieldDesc SPIE = { "SPIE", Id::MSTATUS, {1, 5}, "Previous SIE before the trap"};
213213
static constexpr RegisterFieldDesc MPIE = { "MPIE", Id::MSTATUS, {1, 7}, "Previous MIE before the trap"};
214+
static constexpr RegisterFieldDesc MPRV = { "MPRV", Id::MSTATUS, {1, 17}, "Modify privilege for loads/stores/fetches" };
214215
static constexpr RegisterFieldDesc SPP = { "SPP", Id::MSTATUS, {1, 8}, "System previous privilege mode"};
215216
static constexpr RegisterFieldDesc MPP = { "MPP", Id::MSTATUS, {2, 11}, "Machine previous privilege mode"};
216217
static constexpr RegisterFieldDesc UXL = { "UXL", Id::MSTATUS, {2, 32}, "User mode XLEN (RV64 only)"};

‎src/machine/instruction.cpp‎

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -464,7 +464,7 @@ static const struct InstructionMap SYSTEM_PRIV_map[] = {
464464
IM_UNKNOWN,
465465
IM_UNKNOWN,
466466
IM_UNKNOWN,
467-
{"sret", IT_I, NOALU, NOMEM, nullptr, {}, 0x10200073, 0xffffffff, { .flags = IMF_SUPPORTED | IMF_XRET }, nullptr},
467+
{"sret", IT_I, NOALU, NOMEM, nullptr, {}, 0x10200073, 0xffffffff, { .flags = IMF_SUPPORTED | IMF_XRET | IMF_PRIV_S }, nullptr},
468468
IM_UNKNOWN,
469469
IM_UNKNOWN,
470470
IM_UNKNOWN,
@@ -480,7 +480,7 @@ static const struct InstructionMap SYSTEM_PRIV_map[] = {
480480
IM_UNKNOWN,
481481
IM_UNKNOWN,
482482
IM_UNKNOWN,
483-
{"mret", IT_I, NOALU, NOMEM, nullptr, {}, 0x30200073, 0xffffffff, { .flags = IMF_SUPPORTED | IMF_XRET }, nullptr},
483+
{"mret", IT_I, NOALU, NOMEM, nullptr, {}, 0x30200073, 0xffffffff, { .flags = IMF_SUPPORTED | IMF_XRET | IMF_PRIV_M }, nullptr},
484484
IM_UNKNOWN,
485485
IM_UNKNOWN,
486486
IM_UNKNOWN,
@@ -749,6 +749,27 @@ void Instruction::flags_alu_op_mem_ctl(
749749
flags = (enum InstructionFlags)im.flags;
750750
alu_op = im.alu;
751751
mem_ctl = im.mem_ctl;
752+
if (flags & IMF_CSR) {
753+
machine::CSR::Address csr = csr_address();
754+
uint32_t csr12 = csr.data & 0xfffu;
755+
uint32_t min_bits = (csr12 >> 8) & 0x3u; // csr[9:8]
756+
switch (min_bits) {
757+
case 0u:
758+
// User (no extra flag required)
759+
break;
760+
case 1u:
761+
flags = InstructionFlags(flags | IMF_PRIV_S);
762+
break;
763+
case 2u:
764+
flags = InstructionFlags(flags | IMF_PRIV_H);
765+
break;
766+
case 3u:
767+
flags = InstructionFlags(flags | IMF_PRIV_M);
768+
break;
769+
default:
770+
break;
771+
}
772+
}
752773
}
753774

754775
bool Instruction::operator==(const Instruction &c) const {

‎src/machine/instruction.h‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,11 @@ enum InstructionFlags : unsigned {
6060
// TODO do we want to add those signals to the visualization?
6161

6262
IMF_RV64 = 1L << 24, /**< Mark instructions which are available in 64-bit mode only. */
63+
64+
/* Privilege requirement flags */
65+
IMF_PRIV_S = 1L << 25, /**< Requires at least Supervisor privilege */
66+
IMF_PRIV_H = 1L << 26, /**< Requires at least Hypervisor privilege */
67+
IMF_PRIV_M = 1L << 27, /**< Requires Machine privilege */
6368
};
6469

6570
/**

‎src/machine/pipeline.h‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,8 @@ struct DecodeInterstage {
116116
bool csr = false; // Zicsr, implies csr read and csr write
117117
bool csr_to_alu = false;
118118
bool csr_write = false;
119-
bool xret = false; // Return from exception, MRET and SRET
119+
bool xret = false; // Return from exception, MRET and SRET
120+
CSR::PrivilegeLevel xret_privlev = CSR::PrivilegeLevel::UNPRIVILEGED;
120121
bool insert_stall_before = false;
121122

122123
public:
@@ -179,6 +180,7 @@ struct ExecuteInterstage {
179180
bool csr = false;
180181
bool csr_write = false;
181182
bool xret = false;
183+
CSR::PrivilegeLevel xret_privlev = CSR::PrivilegeLevel::UNPRIVILEGED;
182184

183185
public:
184186
/** Reset to value corresponding to NOP. */
@@ -245,6 +247,7 @@ struct MemoryInterstage {
245247
bool regwrite = false;
246248
bool is_valid = false;
247249
bool csr_written = false;
250+
CSR::PrivilegeLevel xret_privlev = CSR::PrivilegeLevel::UNPRIVILEGED;
248251

249252
public:
250253
/** Reset to value corresponding to NOP. */

0 commit comments

Comments
 (0)