From 9bbd2b397989c4a948b40a7bb40f26644dcefe67 Mon Sep 17 00:00:00 2001 From: Emma Turner Date: Mon, 23 Sep 2024 12:09:38 +0100 Subject: [PATCH] RISC-V: mark control flow instructions as uncacheable --- src/riscv/lib/src/machine_state.rs | 51 ++++++------ src/riscv/lib/src/parser.rs | 35 ++++---- src/riscv/lib/src/parser/instruction.rs | 105 ++++++++++++++---------- src/riscv/lib/src/pvm/sbi.rs | 4 +- 4 files changed, 109 insertions(+), 86 deletions(-) diff --git a/src/riscv/lib/src/machine_state.rs b/src/riscv/lib/src/machine_state.rs index 24ba48b4666d..29d93b961906 100644 --- a/src/riscv/lib/src/machine_state.rs +++ b/src/riscv/lib/src/machine_state.rs @@ -345,13 +345,6 @@ impl MachineCoreStat InstrCacheable::Lhu(args) => run_load_instr!(self, instr, args, run_lhu), InstrCacheable::Lwu(args) => run_load_instr!(self, instr, args, run_lwu), InstrCacheable::Ld(args) => run_load_instr!(self, instr, args, run_ld), - InstrCacheable::Fence(args) => { - self.run_fence(args.pred, args.succ); - Ok(Add(instr.width())) - } - InstrCacheable::FenceTso(_args) => Err(Exception::IllegalInstruction), - InstrCacheable::Ecall => run_syscall_instr!(self, run_ecall), - InstrCacheable::Ebreak => run_syscall_instr!(self, run_ebreak), // RV64I S-type instructions InstrCacheable::Sb(args) => run_store_instr!(self, instr, args, run_sb), @@ -504,20 +497,6 @@ impl MachineCoreStat InstrCacheable::Csrrsi(args) => run_csr_imm_instr!(self, instr, args, run_csrrsi), InstrCacheable::Csrrci(args) => run_csr_imm_instr!(self, instr, args, run_csrrci), - // Privileged instructions - // Trap-Return - InstrCacheable::Mret => run_xret_instr!(self, run_mret), - InstrCacheable::Sret => run_xret_instr!(self, run_sret), - // Currently not implemented instruction (part of Smrnmi extension) - InstrCacheable::Mnret => Err(Exception::IllegalInstruction), - // Interrupt-Management - InstrCacheable::Wfi => run_no_args_instr!(self, instr, run_wfi), - // Supervisor Memory-Management - InstrCacheable::SFenceVma { asid, vaddr } => { - self.run_sfence_vma(*asid, *vaddr)?; - Ok(ProgramCounterUpdate::Add(instr.width())) - } - // RV32C compressed instructions InstrCacheable::CLw(args) => run_load_instr!(self, instr, args, run_clw), InstrCacheable::CLwsp(args) => run_ci_load_sp_instr!(self, instr, args, run_clwsp), @@ -546,7 +525,6 @@ impl MachineCoreStat InstrCacheable::CXor(args) => run_cr_type_instr!(self, instr, args, run_cxor), InstrCacheable::COr(args) => run_cr_type_instr!(self, instr, args, run_cor), InstrCacheable::CSub(args) => run_cr_type_instr!(self, instr, args, run_csub), - InstrCacheable::CEbreak => run_syscall_instr!(self, run_cebreak), InstrCacheable::CNop => { self.run_cnop(); Ok(ProgramCounterUpdate::Add(instr.width())) @@ -712,9 +690,36 @@ impl { + self.core.run_fence(args.pred, args.succ); + Ok(Add(instr.width())) + } + InstrUncacheable::FenceTso(_args) => Err(Exception::IllegalInstruction), + InstrUncacheable::Ecall => run_syscall_instr!(core, run_ecall), + InstrUncacheable::Ebreak => run_syscall_instr!(core, run_ebreak), + + // Privileged instructions + // Trap-Return + InstrUncacheable::Mret => run_xret_instr!(core, run_mret), + InstrUncacheable::Sret => run_xret_instr!(core, run_sret), + // Currently not implemented instruction (part of Smrnmi extension) + InstrUncacheable::Mnret => Err(Exception::IllegalInstruction), + // Interrupt-Management + InstrUncacheable::Wfi => run_no_args_instr!(core, instr, run_wfi), + // Supervisor Memory-Management + InstrUncacheable::SFenceVma { asid, vaddr } => { + self.core.run_sfence_vma(*asid, *vaddr)?; + Ok(ProgramCounterUpdate::Add(instr.width())) + } + + // RV32C compressed instructions + InstrUncacheable::CEbreak => run_syscall_instr!(core, run_cebreak), + // Zifencei instructions InstrUncacheable::FenceI => run_no_args_instr!(self, instr, run_fencei), } diff --git a/src/riscv/lib/src/parser.rs b/src/riscv/lib/src/parser.rs index 79690cf2fd2f..7bf48ae1f4c4 100644 --- a/src/riscv/lib/src/parser.rs +++ b/src/riscv/lib/src/parser.rs @@ -486,6 +486,7 @@ const FM_8: u32 = 0b1000; #[inline] pub const fn parse_uncompressed_instruction(instr: u32) -> Instr { use InstrCacheable::*; + use InstrUncacheable::*; let i = match opcode(instr) { // R-type instructions OP_ARITH => match funct3(instr) { @@ -615,8 +616,8 @@ pub const fn parse_uncompressed_instruction(instr: u32) -> Instr { }, OP_SYNCH => match funct3(instr) { F3_0 => match fm(instr) { - FM_0 => fence_instr!(Fence, instr), - FM_8 => fence_instr!(FenceTso, instr), + FM_0 => return Instr::Uncacheable(fence_instr!(Fence, instr)), + FM_8 => return Instr::Uncacheable(fence_instr!(FenceTso, instr)), _ => Unknown { instr }, }, F3_1 => return Instr::Uncacheable(InstrUncacheable::FenceI), @@ -625,25 +626,27 @@ pub const fn parse_uncompressed_instruction(instr: u32) -> Instr { OP_SYS => match funct3(instr) { F3_0 => match funct7(instr) { F7_0 => match (rs1_bits(instr), rs2_bits(instr)) { - (RS1_0, RS2_0) => Ecall, - (RS1_0, RS2_1) => Ebreak, + (RS1_0, RS2_0) => return Instr::Uncacheable(Ecall), + (RS1_0, RS2_1) => return Instr::Uncacheable(Ebreak), _ => Unknown { instr }, }, - F7_9 => SFenceVma { - vaddr: rs1(instr), - asid: rs2(instr), - }, + F7_9 => { + return Instr::Uncacheable(SFenceVma { + vaddr: rs1(instr), + asid: rs2(instr), + }) + } F7_8 => match (rs1_bits(instr), rs2_bits(instr)) { - (RS1_0, RS2_2) => Sret, - (RS1_0, RS2_5) => Wfi, + (RS1_0, RS2_2) => return Instr::Uncacheable(Sret), + (RS1_0, RS2_5) => return Instr::Uncacheable(Wfi), _ => Unknown { instr }, }, F7_24 => match (rs1_bits(instr), rs2_bits(instr)) { - (RS1_0, RS2_2) => Mret, + (RS1_0, RS2_2) => return Instr::Uncacheable(Mret), _ => Unknown { instr }, }, F7_56 => match (rs1_bits(instr), rs2_bits(instr)) { - (RS1_0, RS2_2) => Mnret, + (RS1_0, RS2_2) => return Instr::Uncacheable(Mnret), _ => Unknown { instr }, }, _ => Unknown { instr }, @@ -1216,7 +1219,7 @@ const fn parse_compressed_instruction_inner(instr: u16) -> Instr { }, C_F3_4 => match (u16::bit(instr, 12), c_rd_rs1(instr), c_rs2(instr)) { - (true, x0, x0) => CEbreak, + (true, x0, x0) => return Instr::Uncacheable(InstrUncacheable::CEbreak), (_, x0, _) => UnknownCompressed { instr }, (true, rs1, x0) => CJalr(CRJTypeArgs { rs1 }), (true, rs1, rs2) => CAdd(CRTypeArgs { rd_rs1: rs1, rs2 }), @@ -1302,8 +1305,8 @@ mod tests { use crate::{ machine_state::{csregisters::CSRegister::mcause, registers::XRegister::*}, parser::{ - instruction::CIBTypeArgs, parse_compressed_instruction, - parse_compressed_instruction_inner, + instruction::{CIBTypeArgs, InstrUncacheable}, + parse_compressed_instruction, parse_compressed_instruction_inner, }, }; @@ -1436,7 +1439,7 @@ mod tests { #[test] fn test_6() { let bytes: [u8; 4] = [0x73, 0x00, 0x20, 0x70]; - let expected = [Instr::Cacheable(Mnret)]; + let expected = [Instr::Uncacheable(InstrUncacheable::Mnret)]; let instructions = parse_block(&bytes); assert_eq!(instructions, expected) } diff --git a/src/riscv/lib/src/parser/instruction.rs b/src/riscv/lib/src/parser/instruction.rs index 99307fa36700..bbf9b58a2e1b 100644 --- a/src/riscv/lib/src/parser/instruction.rs +++ b/src/riscv/lib/src/parser/instruction.rs @@ -269,10 +269,6 @@ pub enum InstrCacheable { Lhu(ITypeArgs), Lwu(ITypeArgs), Ld(ITypeArgs), - Fence(FenceArgs), - FenceTso(FenceArgs), - Ecall, - Ebreak, // RV64I S-type instructions Sb(SBTypeArgs), @@ -409,16 +405,6 @@ pub enum InstrCacheable { Csrrsi(CsriArgs), Csrrci(CsriArgs), - // Privileged instructions - // Trap-Return - Mret, - Sret, - Mnret, - // Interrupt-Management - Wfi, - // Supervisor Memory-Management - SFenceVma { asid: XRegister, vaddr: XRegister }, - // RV32C compressed instructions CLw(ITypeArgs), CLwsp(CIBTypeArgs), @@ -446,7 +432,6 @@ pub enum InstrCacheable { CSub(CRTypeArgs), CAddw(CRTypeArgs), CSubw(CRTypeArgs), - CEbreak, CNop, // RV64C compressed instructions @@ -466,12 +451,39 @@ pub enum InstrCacheable { UnknownCompressed { instr: u16 }, } -/// Instructions that may invalidate the Instruction Cache cannot be -/// stored in that cache, and are split out to statically ensure this. +/// Uncacheable instructions are those that may result in a +/// breaking of the normal flow of execution. +/// +/// Namely, that may happen due: +/// - interrupt control flow +/// - cache invalidation +/// - altering the mapping of virtual to physical memory +/// +/// Any of these can result in breaking the 'default flow of execution', +/// invalidating the assumptions that are required for the [`BlockCache`] to +/// function. #[derive(Debug, PartialEq, Eq, Clone, Copy, EnumTag, Hash)] pub enum InstrUncacheable { + Fence(FenceArgs), + FenceTso(FenceArgs), + Ecall, + Ebreak, + + // RV32C compressed instructions + CEbreak, + // Zifencei instructions FenceI, + + // Privileged instructions + // Trap-Return + Mret, + Sret, + Mnret, + // Interrupt-Management + Wfi, + // Supervisor Memory-Management + SFenceVma { asid: XRegister, vaddr: XRegister }, } /// RISC-V parsed instructions. @@ -529,10 +541,6 @@ impl InstrCacheable { | Lhu(_) | Lwu(_) | Ld(_) - | Fence(_) - | FenceTso(_) - | Ecall - | Ebreak | Sb(_) | Sh(_) | Sw(_) @@ -650,11 +658,6 @@ impl InstrCacheable { | Csrrwi(_) | Csrrsi(_) | Csrrci(_) - | Mret - | Sret - | Mnret - | Wfi - | SFenceVma { .. } | Unknown { instr: _ } => 4, // 2 bytes instructions (compressed instructions) @@ -684,7 +687,6 @@ impl InstrCacheable { | CSub(_) | CAddw(_) | CSubw(_) - | CEbreak | CNop | CLd(_) | CLdsp(_) @@ -705,7 +707,18 @@ impl InstrUncacheable { pub const fn width(&self) -> u64 { use InstrUncacheable::*; match self { - FenceI => 4, + FenceI + | Fence(_) + | FenceTso(_) + | Ecall + | Ebreak + | Mret + | Sret + | Mnret + | Wfi + | SFenceVma { .. } => 4, + + CEbreak => 2, } } } @@ -956,12 +969,6 @@ impl fmt::Display for InstrCacheable { Lwu(args) => i_instr_load!(f, "lwu", args), Ld(args) => i_instr_load!(f, "ld", args), - Fence(args) => fence_instr!(f, "fence", args), - FenceTso(args) => fence_instr!(f, "fence.tso", args), - - Ecall => write!(f, "ecall"), - Ebreak => write!(f, "ebreak"), - // RV64I S-type instructions Sb(args) => s_instr!(f, "sb", args), Sh(args) => s_instr!(f, "sh", args), @@ -1112,16 +1119,6 @@ impl fmt::Display for InstrCacheable { Csrrsi(args) => csri_instr!(f, "csrrsi", args), Csrrci(args) => csri_instr!(f, "csrrci", args), - // Privileged instructions - // Trap-Return - Mret => write!(f, "mret"), - Sret => write!(f, "sret"), - Mnret => write!(f, "mnret"), - // Interrupt-management - Wfi => write!(f, "wfi"), - // Supervisor Memory-Management - SFenceVma { asid, vaddr } => write!(f, "sfence.vma {vaddr},{asid}"), - // RV32C compressed instructions CLw(args) => i_instr_load!(f, "c.lw", args), CLwsp(args) => c_instr_sp!(f, "c.lwsp", args), @@ -1153,7 +1150,6 @@ impl fmt::Display for InstrCacheable { COr(args) => cr_instr!(f, "c.or", args), CXor(args) => cr_instr!(f, "c.xor", args), CSub(args) => cr_instr!(f, "c.sub", args), - CEbreak => write!(f, "c.ebreak"), CNop => write!(f, "c.nop"), // RV64C compressed instructions @@ -1181,8 +1177,27 @@ impl fmt::Display for InstrUncacheable { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { use InstrUncacheable::*; match self { + Fence(args) => fence_instr!(f, "fence", args), + FenceTso(args) => fence_instr!(f, "fence.tso", args), + + Ecall => write!(f, "ecall"), + Ebreak => write!(f, "ebreak"), + + // Privileged instructions + // Trap-Return + Mret => write!(f, "mret"), + Sret => write!(f, "sret"), + Mnret => write!(f, "mnret"), + // Interrupt-management + Wfi => write!(f, "wfi"), + // Supervisor Memory-Management + SFenceVma { asid, vaddr } => write!(f, "sfence.vma {vaddr},{asid}"), + // Zifencei instructions FenceI => write!(f, "fence.i"), + + // Compressed + CEbreak => write!(f, "c.ebreak"), } } } diff --git a/src/riscv/lib/src/pvm/sbi.rs b/src/riscv/lib/src/pvm/sbi.rs index a0dc6a5b8618..6bf93b1809ff 100644 --- a/src/riscv/lib/src/pvm/sbi.rs +++ b/src/riscv/lib/src/pvm/sbi.rs @@ -10,7 +10,7 @@ use crate::{ registers::{a0, a1, a2, a3, a6, a7, XValue}, AccessType, MachineState, }, - parser::instruction::InstrCacheable, + parser::instruction::InstrUncacheable, state_backend::{CellRead, CellReadWrite, CellWrite, ManagerReadWrite}, traps::EnvironException, }; @@ -417,7 +417,7 @@ where // No matter the outcome, we need to bump the // program counter because ECALL's don't update it // to the following instructions. - let pc = machine.core.hart.pc.read() + InstrCacheable::Ecall.width(); + let pc = machine.core.hart.pc.read() + InstrUncacheable::Ecall.width(); machine.core.hart.pc.write(pc); // SBI extension is contained in a7. -- GitLab