Deduplicate set_mpp and get_raw_faulting_instr

Both instructions performs exactly the same logic in userspace.rs and metal.rs and are not architecture dependent. This commit removes the smell

src/arch/metal.rs

@@ -1,14 +1,13 @@
 //! Bare metal RISC-V
 use core::arch::{asm, global_asm};
 use core::marker::PhantomData;
-use core::{ptr, usize};
+use core::usize;
 
-use super::{
-    menvcfg, Arch, Architecture, Csr, ExtensionsCapability, MCause, Mode, RegistersCapability,
-    TrapInfo,
-};
+use super::{menvcfg, Arch, Architecture, Csr, ExtensionsCapability, Mode, RegistersCapability};
 use crate::arch::pmp::PmpFlush;
-use crate::arch::{mie, misa, mstatus, parse_mpp_return_mode, HardwareCapability, PmpGroup, Width};
+use crate::arch::{
+    mie, misa, mstatus, parse_mpp_return_mode, set_mpp, HardwareCapability, PmpGroup, Width,
+};
 use crate::decoder::Instr;
 use crate::virt::VirtContext;
 use crate::{utils, RegisterContextGetter, RegisterContextSetter};
@@ -375,13 +374,6 @@ impl Architecture for MetalArch {
         }
     }
 
-    unsafe fn set_mpp(mode: Mode) -> Mode {
-        let value = mode.to_bits() << mstatus::MPP_OFFSET;
-        let prev_mstatus = Self::read_csr(Csr::Mstatus);
-        Self::write_csr(Csr::Mstatus, (prev_mstatus & !mstatus::MPP_FILTER) | value);
-        parse_mpp_return_mode(prev_mstatus)
-    }
-
     unsafe fn write_pmp(pmp: &PmpGroup) -> PmpFlush {
         let pmpaddr = pmp.pmpaddr();
         let pmpcfg = pmp.pmpcfg();
@@ -403,22 +395,6 @@ impl Architecture for MetalArch {
         PmpFlush()
     }
 
-    unsafe fn get_raw_faulting_instr(trap_info: &TrapInfo) -> usize {
-        if trap_info.mcause == MCause::IllegalInstr as usize {
-            // First, try mtval and check if it contains an instruction
-            if trap_info.mtval != 0 {
-                return trap_info.mtval;
-            }
-        }
-
-        let instr_ptr = trap_info.mepc as *const u32;
-
-        // With compressed instruction extention ("C") instructions can be misaligned.
-        // TODO: add support for 16 bits instructions
-        let instr = ptr::read_unaligned(instr_ptr);
-        instr as usize
-    }
-
     unsafe fn run_vcpu(ctx: &mut VirtContext) {
         asm!(
             // We need to save some registers manually, the compiler can't handle those
@@ -760,7 +736,7 @@ impl Architecture for MetalArch {
         Self::write_csr(Csr::Mcause, 0);
 
         // Set the MPP mode to match the vMPP
-        let prev_mpp = Self::set_mpp(parse_mpp_return_mode(ctx.csr.mstatus));
+        let prev_mpp = set_mpp(parse_mpp_return_mode(ctx.csr.mstatus));
         let prev_satp = Self::write_csr(Csr::Satp, ctx.csr.satp);
 
         // Changes to SATP require an sfence instruction to take effect
@@ -877,7 +853,7 @@ impl Architecture for MetalArch {
 
         // Restore the original values
         Self::write_csr(Csr::Satp, prev_satp);
-        Self::set_mpp(prev_mpp);
+        set_mpp(prev_mpp);
 
         // Ensure memory consistency
         Self::sfencevma(None, None);
@@ -893,7 +869,7 @@ impl Architecture for MetalArch {
         let prev_mstatus = Self::read_csr(Csr::Mstatus);
 
         // Set mstatus.MPP to mode
-        let prev_mode = Self::set_mpp(mode);
+        let prev_mode = set_mpp(mode);
         for i in 0..dest.len() {
             let mut byte_read: u8 = 0;
             unsafe {
@@ -941,7 +917,7 @@ impl Architecture for MetalArch {
             src += 1;
         }
 
-        Self::set_mpp(prev_mode);
+        set_mpp(prev_mode);
         Ok(())
     }
 
@@ -955,7 +931,7 @@ impl Architecture for MetalArch {
         let prev_mstatus = Self::read_csr(Csr::Mstatus);
 
         // Set mstatus.MPP to mode
-        let prev_mode = Self::set_mpp(mode);
+        let prev_mode = set_mpp(mode);
         for i in 0..src.len() {
             let byte_value: u8 = src[i];
             unsafe {
@@ -1000,7 +976,7 @@ impl Architecture for MetalArch {
             dest += 1;
         }
 
-        Self::set_mpp(prev_mode);
+        set_mpp(prev_mode);
         Ok(())
     }
 }

src/arch/mod.rs

@@ -17,6 +17,8 @@ mod registers;
 mod trap;
 pub mod userspace;
 
+use core::ptr;
+
 use pmp::{PmpFlush, PmpGroup};
 pub use registers::{Csr, Register};
 pub use trap::{MCause, TrapInfo};
@@ -69,8 +71,6 @@ pub trait Architecture {
     /// environment.
     unsafe fn set_csr_bits(csr: Csr, bits_mask: usize) -> usize;
 
-    /// Change mstatus.MPP and return the previous mstatus.MPP
-    unsafe fn set_mpp(mode: Mode) -> Mode;
     unsafe fn write_pmp(pmp: &PmpGroup) -> PmpFlush;
     unsafe fn sfencevma(vaddr: Option<usize>, asid: Option<usize>);
     unsafe fn hfencegvma(vaddr: Option<usize>, asid: Option<usize>);
@@ -95,13 +95,6 @@ pub trait Architecture {
     /// It should not be assume that any of the core configuration is preserved by this function.
     unsafe fn detect_hardware() -> HardwareCapability;
 
-    /// Return the faulting instruction at the provided exception PC.
-    ///
-    /// # Safety
-    ///
-    /// The trap info must correspond to a valid trap info, no further checks are performed.
-    unsafe fn get_raw_faulting_instr(trap_info: &TrapInfo) -> usize;
-
     unsafe fn handle_virtual_load_store(instr: Instr, ctx: &mut VirtContext);
 
     /// Copies dest.len() bytes from src to dest, using the provided mode to read from src.
@@ -517,3 +510,34 @@ impl From<usize> for Width {
         }
     }
 }
+
+// ———————————————————————— Helpers ————————————————————————— //
+
+/// Change mstatus.MPP and return the previous mstatus.MPP
+pub unsafe fn set_mpp(mode: Mode) -> Mode {
+    let value = mode.to_bits() << mstatus::MPP_OFFSET;
+    let prev_mstatus = Arch::read_csr(Csr::Mstatus);
+    Arch::write_csr(Csr::Mstatus, (prev_mstatus & !mstatus::MPP_FILTER) | value);
+    parse_mpp_return_mode(prev_mstatus)
+}
+
+/// Return the faulting instruction at the provided exception PC.
+///
+/// # Safety
+///
+/// The trap info must correspond to a valid trap info, no further checks are performed.
+pub unsafe fn get_raw_faulting_instr(trap_info: &TrapInfo) -> usize {
+    if trap_info.mcause == MCause::IllegalInstr as usize {
+        // First, try mtval and check if it contains an instruction
+        if trap_info.mtval != 0 {
+            return trap_info.mtval;
+        }
+    }
+
+    let instr_ptr = trap_info.mepc as *const u32;
+
+    // With compressed instruction extention ("C") instructions can be misaligned.
+    // TODO: add support for 16 bits instructions
+    let instr = ptr::read_unaligned(instr_ptr);
+    instr as usize
+}

src/arch/userspace.rs

@@ -4,13 +4,10 @@
 //! architecture, such as when running unit tests.
 
 use core::marker::PhantomData;
-use core::ptr;
 
 use spin::{Mutex, MutexGuard};
 
-use super::{
-    mie, mstatus, parse_mpp_return_mode, Architecture, Csr, ExtensionsCapability, MCause, Mode,
-};
+use super::{mie, mstatus, Architecture, Csr, ExtensionsCapability, Mode};
 use crate::arch::pmp::PmpFlush;
 use crate::arch::{HardwareCapability, PmpGroup};
 use crate::decoder::Instr;
@@ -47,13 +44,6 @@ impl Architecture for HostArch {
         log::debug!("Userspace wfi");
     }
 
-    unsafe fn set_mpp(mode: Mode) -> Mode {
-        let value = mode.to_bits() << mstatus::MPP_OFFSET;
-        let prev_mstatus = Self::read_csr(Csr::Mstatus);
-        Self::write_csr(Csr::Mstatus, (prev_mstatus & !mstatus::MPP_FILTER) | value);
-        parse_mpp_return_mode(prev_mstatus)
-    }
-
     unsafe fn write_pmp(pmp: &PmpGroup) -> PmpFlush {
         let pmpaddr = pmp.pmpaddr();
         let pmpcfg = pmp.pmpcfg();
@@ -78,22 +68,6 @@ impl Architecture for HostArch {
         todo!()
     }
 
-    unsafe fn get_raw_faulting_instr(trap_info: &super::TrapInfo) -> usize {
-        if trap_info.mcause == MCause::IllegalInstr as usize {
-            // First, try mtval and check if it contains an instruction
-            if trap_info.mtval != 0 {
-                return trap_info.mtval;
-            }
-        }
-
-        let instr_ptr = trap_info.mepc as *const u32;
-
-        // With compressed instruction extention ("C") instructions can be misaligned.
-        // TODO: add support for 16 bits instructions
-        let instr = ptr::read_unaligned(instr_ptr);
-        instr as usize
-    }
-
     unsafe fn sfencevma(_vaddr: Option<usize>, _asid: Option<usize>) {
         log::debug!("Userspace sfencevma");
     }

src/main.rs

@@ -12,7 +12,7 @@
 
 use core::arch::global_asm;
 
-use miralis::arch::{misa, Arch, Architecture, Csr, Mode, Register};
+use miralis::arch::{misa, set_mpp, Arch, Architecture, Csr, Mode, Register};
 use miralis::config::{
     DELEGATE_PERF_COUNTER, PLATFORM_BOOT_HART_ID, PLATFORM_NAME, PLATFORM_NB_HARTS,
     TARGET_STACK_SIZE,
@@ -66,7 +66,7 @@ pub(crate) extern "C" fn main(_hart_id: usize, device_tree_blob_addr: usize) ->
     let mut ctx = VirtContext::new(hart_id, mctx.pmp.nb_virt_pmp, mctx.hw.extensions.clone());
     unsafe {
         // Set return address, mode and PMP permissions
-        Arch::set_mpp(Mode::U);
+        set_mpp(Mode::U);
         // Update the PMPs prior to first entry
         Arch::write_pmp(&mctx.pmp).flush();
 

src/policy/keystone.rs

@@ -9,7 +9,9 @@ use core::ptr;
 
 use crate::arch::pmp::pmplayout::POLICY_OFFSET;
 use crate::arch::pmp::{pmpcfg, Segment};
-use crate::arch::{parse_mpp_return_mode, Arch, Architecture, Csr, MCause, Mode, Register};
+use crate::arch::{
+    parse_mpp_return_mode, set_mpp, Arch, Architecture, Csr, MCause, Mode, Register,
+};
 use crate::host::MiralisContext;
 use crate::policy::{PolicyHookResult, PolicyModule};
 use crate::virt::traits::*;
@@ -135,7 +137,7 @@ impl EnclaveCtx {
 
             // Swap M-mode registers
             self.mideleg = Arch::write_csr(Csr::Mideleg, self.mideleg);
-            self.mpp = Arch::set_mpp(self.mpp);
+            self.mpp = set_mpp(self.mpp);
         }
     }
 }

src/virt/emulator.rs

@@ -8,8 +8,8 @@ use crate::arch::mie::{
     MEIE_OFFSET, MSIE_OFFSET, MTIE_OFFSET, SEIE_OFFSET, SSIE_OFFSET, STIE_OFFSET,
 };
 use crate::arch::{
-    mie, misa, mstatus, mtvec, parse_mpp_return_mode, Arch, Architecture, Csr, MCause, Mode,
-    Register,
+    get_raw_faulting_instr, mie, misa, mstatus, mtvec, parse_mpp_return_mode, Arch, Architecture,
+    Csr, MCause, Mode, Register,
 };
 use crate::benchmark::Benchmark;
 use crate::decoder::Instr;
@@ -357,7 +357,7 @@ impl VirtContext {
                 panic!("Firmware should not be able to come from S-mode");
             }
             MCause::IllegalInstr => {
-                let instr = unsafe { Arch::get_raw_faulting_instr(&self.trap_info) };
+                let instr = unsafe { get_raw_faulting_instr(&self.trap_info) };
                 let instr = mctx.decode(instr);
                 if logger::trace_enabled!() {
                     log::trace!("Faulting instruction: {:?}", instr);
@@ -372,7 +372,7 @@ impl VirtContext {
                 if let Some(device) =
                     device::find_matching_device(self.trap_info.mtval, mctx.devices)
                 {
-                    let instr = unsafe { Arch::get_raw_faulting_instr(&self.trap_info) };
+                    let instr = unsafe { get_raw_faulting_instr(&self.trap_info) };
                     let instr = mctx.decode(instr);
                     log::trace!(
                         "Accessed devices: {} | With instr: {:?}",
@@ -382,7 +382,7 @@ impl VirtContext {
                     self.handle_device_access_fault(&instr, device);
                 } else if (self.csr.mstatus & mstatus::MPRV_FILTER) >> mstatus::MPRV_OFFSET == 1 {
                     // TODO: make sure virtual address does not get around PMP protection
-                    let instr = unsafe { Arch::get_raw_faulting_instr(&self.trap_info) };
+                    let instr = unsafe { get_raw_faulting_instr(&self.trap_info) };
                     let instr = mctx.decode(instr);
                     log::trace!(
                         "Access fault {:x?} with a virtual address: 0x{:x}",

src/virt/world_switch.rs

@@ -5,7 +5,7 @@
 use super::{VirtContext, VirtCsr};
 use crate::arch::pmp::pmpcfg;
 use crate::arch::pmp::pmpcfg::NO_PERMISSIONS;
-use crate::arch::{mie, mstatus, Arch, Architecture, Csr, Mode};
+use crate::arch::{mie, mstatus, set_mpp, Arch, Architecture, Csr, Mode};
 use crate::config::DELEGATE_PERF_COUNTER;
 use crate::host::MiralisContext;
 
@@ -120,7 +120,7 @@ impl VirtContext {
 
         self.csr.mstatus = self.csr.mstatus & !mstatus::SSTATUS_FILTER
             | Arch::read_csr(Csr::Mstatus) & mstatus::SSTATUS_FILTER;
-        Arch::set_mpp(Mode::U);
+        set_mpp(Mode::U);
         Arch::write_csr(Csr::Mideleg, 0); // Do not delegate any interrupts
         Arch::write_csr(Csr::Medeleg, 0); // Do not delegate any exceptions