feat: Emu builder pattern

.github/workflows/rust_ci.yaml

@@ -69,7 +69,7 @@ jobs:
   cargo-lint:
     runs-on: ubuntu-latest
     timeout-minutes: 20
-    name: Clippy
+    name: Clippy (feature powerset)
     steps:
       - name: Checkout sources
         uses: actions/checkout@v4

Cargo.toml

@@ -25,7 +25,6 @@ unnameable-types = "warn"
 all = "warn"
 
 [workspace.lints.clippy]
-needless-return = "allow" # Temporary fix since this is breaking in nightly clippy
 all = { level = "warn", priority = -1 }
 missing-const-for-fn = "warn"
 use-self = "warn"
@@ -42,7 +41,6 @@ brisc-emu = { path = "crates/hw", default-features = false }
 # External
 cfg-if = "1.0.0"
 hashbrown = "0.15"
-bitflags = { version = "2.6.0", default-features = false }
 thiserror = { version = "2.0.11", default-features = false }
 num-traits = { version = "0.2", default-features = false }
 

crates/emu/src/cfg.rs

@@ -1,15 +1,12 @@
-//! Emulator configuration trait.
+//! Emulator type configuration trait
 
-use brisc_hw::{linux::SyscallInterface, memory::Memory};
+use brisc_hw::{kernel::Kernel, memory::Memory};
 
-#[allow(unused, missing_docs)]
+/// The [`EmuConfig`] trait defines the type configuration for the emulator.
 pub trait EmuConfig {
     /// The [Memory] type used by the emulator.
     type Memory: Memory;
 
-    /// The system call interface used by the emulator.
-    type SyscallInterface: SyscallInterface;
-
-    /// External configuration for the emulator.
-    type ExternalConfig;
+    /// The kernel used by the emulator.
+    type Kernel: Kernel;
 }

crates/emu/src/elf/load.rs

@@ -1,6 +1,5 @@
 //! ELF file loading utilities.
 
-use crate::{cfg::EmuConfig, st::StEmu};
 use alloc::{
     format,
     string::{String, ToString},
@@ -11,21 +10,21 @@ use brisc_hw::{
 };
 use elf::{abi::PT_LOAD, endian::AnyEndian, ElfBytes};
 
-/// Load a raw ELF file into a [StEmu] object.
+/// Load a raw ELF file into a fresh instance of [`Memory`].
 ///
 /// ### Takes
 /// - `raw`: The raw contents of the ELF file to load.
 ///
 /// ### Returns
-/// - `Ok(state)` if the ELF file was loaded successfully
+/// - `Ok((memory, entry_pc))` if the ELF file was loaded successfully
 /// - `Err(_)` if the ELF file could not be loaded
-pub fn load_elf<Config>(raw: &[u8]) -> Result<StEmu<Config>, String>
+pub fn load_elf<M>(raw: &[u8]) -> Result<(M, XWord), String>
 where
-    Config: EmuConfig<Memory: Default, SyscallInterface: Default>,
+    M: Memory + Default,
 {
     let elf = ElfBytes::<AnyEndian>::minimal_parse(raw)
         .map_err(|e| format!("Failed to parse ELF file: {e}"))?;
-    let mut memory = Config::Memory::default();
+    let mut memory = M::default();
 
     let headers = elf.segments().ok_or("Failed to load section headers")?;
     for (i, header) in headers.iter().enumerate() {
@@ -73,7 +72,5 @@ where
             .map_err(|e| e.to_string())?;
     }
 
-    // TODO: Allow for passing a syscall interface, _or_ just make this function
-    // initialize the memory.
-    Ok(StEmu::new(elf.ehdr.e_entry as XWord, memory, Config::SyscallInterface::default()))
+    Ok((memory, elf.ehdr.e_entry as XWord))
 }

crates/emu/src/st/builder.rs

@@ -0,0 +1,79 @@
+//! A builder for the [`StEmu`] emulator.
+
+use super::StEmu;
+use crate::{cfg::EmuConfig, elf::load_elf};
+use alloc::string::String;
+use brisc_hw::{pipeline::PipelineRegister, XWord};
+
+/// A builder for the [`StEmu`] emulator.
+#[derive(Debug)]
+pub struct StEmuBuilder<Config>
+where
+    Config: EmuConfig,
+{
+    /// The starting program counter.
+    pub pc: XWord,
+    /// The initial memory for the emulator.
+    pub memory: Option<Config::Memory>,
+    /// The system call interface for the emulator.
+    pub kernel: Option<Config::Kernel>,
+}
+
+impl<Config> Default for StEmuBuilder<Config>
+where
+    Config: EmuConfig,
+{
+    fn default() -> Self {
+        Self { pc: 0, memory: None, kernel: None }
+    }
+}
+
+impl<Config> StEmuBuilder<Config>
+where
+    Config: EmuConfig,
+    Config::Memory: Default,
+{
+    /// Loads an elf file into the emulator builder, initializing the program counter and memory.
+    pub fn with_elf(mut self, elf_bytes: &[u8]) -> Result<Self, String> {
+        let (memory, entry_pc) = load_elf::<Config::Memory>(elf_bytes)?;
+        self.pc = entry_pc;
+        self.memory = Some(memory);
+        Ok(self)
+    }
+}
+
+impl<Config> StEmuBuilder<Config>
+where
+    Config: EmuConfig,
+{
+    /// Assigns the entry point of the program.
+    pub const fn with_pc(mut self, pc: XWord) -> Self {
+        self.pc = pc;
+        self
+    }
+
+    /// Assigns a pre-created memory instance to the emulator.
+    pub fn with_memory(mut self, memory: Config::Memory) -> Self {
+        self.memory = Some(memory);
+        self
+    }
+
+    /// Assigns the kernel to the emulator.
+    pub fn with_kernel(mut self, kernel: Config::Kernel) -> Self {
+        self.kernel = Some(kernel);
+        self
+    }
+
+    /// Builds the emulator with the current configuration.
+    ///
+    /// ## Panics
+    ///
+    /// Panics if the memory or kernel is not set.
+    pub fn build(self) -> StEmu<Config> {
+        StEmu {
+            register: PipelineRegister::new(self.pc),
+            memory: self.memory.expect("Memory not instantiated"),
+            kernel: self.kernel.expect("Kernel not instantiated"),
+        }
+    }
+}

crates/emu/src/st/mod.rs

@@ -3,13 +3,15 @@
 use crate::cfg::EmuConfig;
 use brisc_hw::{
     errors::{PipelineError, PipelineResult},
-    linux::SyscallInterface,
+    kernel::Kernel,
     pipeline::{
         decode_instruction, execute, instruction_fetch, mem_access, writeback, PipelineRegister,
     },
-    XWord,
 };
 
+mod builder;
+pub use builder::StEmuBuilder;
+
 /// Single-cycle RISC-V processor emulator.
 #[derive(Debug, Default)]
 pub struct StEmu<Config>
@@ -21,22 +23,16 @@ where
     /// The device memory.
     pub memory: Config::Memory,
     /// The system call interface.
-    pub syscall_interface: Config::SyscallInterface,
+    pub kernel: Config::Kernel,
 }
 
 impl<Config> StEmu<Config>
 where
     Config: EmuConfig,
 {
-    /// Create a new [StEmu] with the given [Memory] and [SyscallInterface].
-    ///
-    /// [Memory]: brisc_hw::memory::Memory
-    pub fn new(
-        pc: XWord,
-        memory: Config::Memory,
-        syscall_interface: Config::SyscallInterface,
-    ) -> Self {
-        Self { register: PipelineRegister::new(pc), memory, syscall_interface }
+    /// Creates a new [`StEmuBuilder`].
+    pub fn builder() -> StEmuBuilder<Config> {
+        StEmuBuilder::default()
     }
 
     /// Executes the program until it exits, returning the final [PipelineRegister].
@@ -49,6 +45,7 @@ where
     }
 
     /// Execute a single cycle of the processor in full.
+    #[inline(always)]
     pub fn cycle(&mut self) -> PipelineResult<()> {
         let r = &mut self.register;
 
@@ -61,16 +58,16 @@ where
 
         // Handle system calls.
         match cycle_res {
+            Ok(()) => {}
             Err(PipelineError::SyscallException(syscall_no)) => {
-                self.syscall_interface.syscall(syscall_no, &mut self.memory, r)?;
+                self.kernel.syscall(syscall_no, &mut self.memory, r)?;
 
                 // Exit emulation if the syscall terminated the program.
                 if r.exit {
                     return Ok(());
                 }
             }
             Err(e) => return Err(e),
-            _ => {}
         }
 
         r.advance();

crates/emu/src/test_utils.rs

@@ -1,10 +1,11 @@
 //! Test utilities for the emulator crate.
 
-use crate::{cfg::EmuConfig, elf::load_elf};
+use crate::{cfg::EmuConfig, st::StEmu};
 use brisc_hw::{
     errors::PipelineResult,
-    linux::SyscallInterface,
+    kernel::Kernel,
     memory::{Memory, SimpleMemory},
+    pipeline::PipelineRegister,
     XWord, REG_A0,
 };
 use rstest as _;
@@ -36,7 +37,11 @@ macro_rules! test_suites {
 pub fn run_riscv_test(test_path: &PathBuf) -> f64 {
     // Load the program
     let elf_bytes = fs::read(test_path).unwrap();
-    let mut hart = load_elf::<TestStEmuConfig>(&elf_bytes).unwrap();
+    let mut hart = StEmu::<TestStEmuConfig>::builder()
+        .with_kernel(RiscvTestKernel)
+        .with_elf(&elf_bytes)
+        .unwrap()
+        .build();
 
     // Run the program until it exits
     let mut clock = 0;
@@ -45,9 +50,8 @@ pub fn run_riscv_test(test_path: &PathBuf) -> f64 {
         hart.cycle().unwrap();
         clock += 1;
     }
-    let elapsed = now.elapsed();
 
-    let ips = clock as f64 / elapsed.as_secs_f64();
+    let ips = clock as f64 / now.elapsed().as_secs_f64();
     println!("ips: {ips}");
 
     // Check the exit code
@@ -68,20 +72,18 @@ struct TestStEmuConfig;
 impl EmuConfig for TestStEmuConfig {
     type Memory = SimpleMemory;
 
-    type SyscallInterface = RiscvTestSyscalls;
-
-    type ExternalConfig = ();
+    type Kernel = RiscvTestKernel;
 }
 
 #[derive(Default)]
-struct RiscvTestSyscalls;
+struct RiscvTestKernel;
 
-impl SyscallInterface for RiscvTestSyscalls {
+impl Kernel for RiscvTestKernel {
     fn syscall<M: Memory>(
         &mut self,
         sysno: XWord,
         _: &mut M,
-        p_reg: &mut brisc_hw::pipeline::PipelineRegister,
+        p_reg: &mut PipelineRegister,
     ) -> PipelineResult<XWord> {
         match sysno {
             0x5D => {

crates/hw/Cargo.toml

@@ -17,7 +17,6 @@ workspace = true
 brisc-isa.workspace = true
 
 # External
-bitflags.workspace = true
 thiserror.workspace = true
 hashbrown.workspace = true
 cfg-if.workspace = true

crates/hw/src/linux.rs → crates/hw/src/kernel.rs

@@ -1,10 +1,10 @@
-//! Linux system call interface.
+//! Linux kernel interface.
 
 use crate::{errors::PipelineResult, memory::Memory, pipeline::PipelineRegister};
 use brisc_isa::XWord;
 
-/// The [SyscallInterface] trait defines the interface for performing system calls.
-pub trait SyscallInterface {
+/// The [`Kernel`] trait defines the interface for performing system calls.
+pub trait Kernel {
     /// Perform a system call with the given arguments.
     fn syscall<M: Memory>(
         &mut self,
@@ -14,7 +14,7 @@ pub trait SyscallInterface {
     ) -> PipelineResult<XWord>;
 }
 
-impl SyscallInterface for () {
+impl Kernel for () {
     fn syscall<M: Memory>(
         &mut self,
         _: XWord,

crates/hw/src/lib.rs

@@ -8,7 +8,7 @@
 extern crate alloc;
 
 pub mod errors;
-pub mod linux;
+pub mod kernel;
 pub mod memory;
 pub mod pipeline;
 

crates/hw/src/pipeline/decode.rs

@@ -23,8 +23,6 @@ pub fn decode_instruction(register: &mut PipelineRegister) -> PipelineResult<()>
     // Set the decoded instruction in the pipeline register.
     register.instruction = Some(instruction);
 
-    // TODO: Assign control signals.
-
     // Throw an interrupt if the instruction is a system call.
     if instruction.is_system_call() {
         return Err(PipelineError::SyscallException(register.registers[REG_A7 as usize]));

crates/hw/src/pipeline/execute.rs

@@ -49,8 +49,7 @@ pub fn execute(p_reg: &mut PipelineRegister) -> PipelineResult<()> {
         }
         Instruction::Environment(_i_type, funct) => {
             if matches!(funct, EnvironmentFunction::Ecall) {
-                // TODO: Fix; Needs to be a0
-                return Err(PipelineError::SyscallException(p_reg.registers[0]));
+                unreachable!("Ecall should be handled in the decode stage");
             } else {
                 // no-op EBREAK operations.
                 0